-
-
Notifications
You must be signed in to change notification settings - Fork 274
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update drop drown for logged in desktop (library) #2021
base: modularization-main
Are you sure you want to change the base?
Changes from all commits
1b6585a
8deea5f
5c14761
5fb7fec
7dc05f1
22308f8
2317486
ac5353b
e09d442
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,22 +18,74 @@ import {Autocomplete} from './Autocomplete' | |
import { DropdownMenu, DropdownMenuSeparator, DropdownMenuItem, DropdownMenuItemWithIcon } from './common/DropdownMenu'; | ||
|
||
|
||
const LoggedInDropdown = () => { | ||
|
||
const getCurrentPage = () => { | ||
return encodeURIComponent(Sefaria.util.currentPath()); | ||
} | ||
|
||
return ( | ||
<DropdownMenu menuIconComponent={<ProfilePic | ||
url={Sefaria.profile_pic_url} | ||
name={Sefaria.full_name} | ||
len={25} | ||
/> | ||
}> | ||
<DropdownMenuItem> | ||
<strong>{Sefaria.full_name}</strong> | ||
</DropdownMenuItem> | ||
<DropdownMenuSeparator /> | ||
<DropdownMenuItem url={'/settings/account'}> | ||
<InterfaceText>Account Settings</InterfaceText> | ||
</DropdownMenuItem> | ||
<DropdownMenuItem url={'/torahtracker'}> | ||
<InterfaceText text={{'en': 'Torah Tracker', 'he': 'לימוד במספרים'}} /> | ||
</DropdownMenuItem> | ||
<DropdownMenuSeparator /> | ||
<div className="languageHeader"> | ||
<InterfaceText>Site Language</InterfaceText> | ||
</div> | ||
<div className='languageToggleFlexContainer'> | ||
<DropdownMenuItem url={`/interface/english?next=${getCurrentPage()}`}> | ||
English | ||
</DropdownMenuItem> | ||
<span className="languageDot">·</span> | ||
<DropdownMenuItem url={`/interface/hebrew?next=${getCurrentPage()}`}> | ||
עברית | ||
</DropdownMenuItem> | ||
</div> | ||
<DropdownMenuSeparator /> | ||
<DropdownMenuItem url={'/notifications'}> | ||
<InterfaceText text={{'en': 'New Additions', 'he': 'חידושים בארון הספרים של ספריא'}} /> | ||
</DropdownMenuItem> | ||
<DropdownMenuItem url={'/help'}> | ||
<InterfaceText text={{'en': 'Help', 'he': 'עזרה'}} /> | ||
</DropdownMenuItem> | ||
<DropdownMenuSeparator/> | ||
<DropdownMenuItem url={'/logout'}> | ||
<InterfaceText text={{'en': 'Log Out', 'he': 'ניתוק'}} /> | ||
</DropdownMenuItem> | ||
</DropdownMenu> | ||
); | ||
} | ||
|
||
|
||
const ModuleSwitcher = () => { | ||
return ( | ||
<DropdownMenu> | ||
<DropdownMenuItem url={'/'}> | ||
<DropdownMenu menuIconComponent={<img src='/static/icons/module_switcher_icon.svg'/>}> | ||
<DropdownMenuItem url={'/'} newTab={true}> | ||
<DropdownMenuItemWithIcon icon={'/static/icons/library_icon.svg'} textEn={'Library'} textHe={'ספריה'} /> | ||
</DropdownMenuItem> | ||
<DropdownMenuSeparator /> | ||
<DropdownMenuItem url={'//sheets.sefaria.org'}> | ||
<DropdownMenuItem url={'//sheets.sefaria.org'} newTab={true}> | ||
<DropdownMenuItemWithIcon icon={'/static/icons/sheets_icon.svg'} textEn={'Sheets'} textHe={'דפים'}/> | ||
</DropdownMenuItem> | ||
<DropdownMenuSeparator /> | ||
<DropdownMenuItem url={'//developers.sefaria.org'}> | ||
<DropdownMenuItem url={'//developers.sefaria.org'} newTab={true}> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Switch to full URL |
||
<DropdownMenuItemWithIcon icon={'/static/icons/developers_icon.svg'} textEn={'Developers'} textHe={'מפתחים'}/> | ||
</DropdownMenuItem> | ||
<DropdownMenuSeparator /> | ||
<DropdownMenuItem url={'//sefaria.org/products'}> | ||
<DropdownMenuItem url={'//sefaria.org/products'} newTab={true}> | ||
<InterfaceText text={{'he':'לכל המוצרים שלנו', 'en': 'See all products ›'}} /> | ||
</DropdownMenuItem> | ||
|
||
|
@@ -87,21 +139,20 @@ class Header extends Component { | |
openURL={this.props.openURL} | ||
/> | ||
|
||
{ !Sefaria._uid && Sefaria._siteSettings.TORAH_SPECIFIC ? | ||
<InterfaceLanguageMenu | ||
currentLang={Sefaria.interfaceLang} | ||
translationLanguagePreference={this.props.translationLanguagePreference} | ||
setTranslationLanguagePreference={this.props.setTranslationLanguagePreference} /> : null} | ||
|
||
|
||
<ModuleSwitcher /> | ||
|
||
{ Sefaria._uid ? | ||
<LoggedInButtons headerMode={this.props.headerMode}/> | ||
<LoggedInDropdown /> | ||
: <LoggedOutButtons headerMode={this.props.headerMode}/> | ||
} | ||
|
||
{ !Sefaria._uid && Sefaria._siteSettings.TORAH_SPECIFIC ? | ||
<ModuleSwitcher /> : null} | ||
|
||
{ !Sefaria._uid && Sefaria._siteSettings.TORAH_SPECIFIC ? | ||
<InterfaceLanguageMenu | ||
currentLang={Sefaria.interfaceLang} | ||
translationLanguagePreference={this.props.translationLanguagePreference} | ||
setTranslationLanguagePreference={this.props.setTranslationLanguagePreference} /> : null} | ||
|
||
</div> | ||
</> | ||
); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,10 +14,15 @@ const DropdownMenuSeparator = () => { | |
|
||
} | ||
|
||
const DropdownMenuItem = ({url, children}) => { | ||
const DropdownMenuItem = ({url, children, newTab}) => { | ||
|
||
if (!newTab){ | ||
newTab = false; | ||
} | ||
|
||
return ( | ||
|
||
<a className={`interfaceLinks-option int-bi dropdownItem`} href={url} target="_blank"> | ||
<a className={`interfaceLinks-option int-bi dropdownItem`} href={url} target={newTab ? '_blank' : null}> | ||
{children} | ||
</a> | ||
|
||
|
@@ -40,7 +45,7 @@ const DropdownMenuItemWithIcon = ({icon, textEn, textHe}) => { | |
); | ||
} | ||
|
||
const DropdownMenu = ({children}) => { | ||
const DropdownMenu = ({children, menuIconComponent}) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What are you passing in here? another react component as a prop? thats generally not considered a good practice... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you need to be able to insert a dynamic component the way to do it is composability, as we have done with the rest of the menu.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for pointing this out! I didn't realize, and really appreciate learning about better React practices. I will push back a drop - in this case, I needed parity with the code that renders the "initials" when a profile image is not present, and needed to structure the code this way iirc to enable that in both PRs. Happy to discuss more, and hear your insights as to if there's a better and more proper React way of doing this. Thanks! |
||
const [isOpen, setIsOpen] = useState(false); | ||
const wrapperRef = useRef(null); | ||
|
||
|
@@ -73,7 +78,9 @@ const DropdownMenu = ({children}) => { | |
|
||
return ( | ||
<div className="dropdownLinks" ref={wrapperRef}> | ||
<a className="dropdownLinks-button" onClick={handleClick}><img src="/static/icons/module_switcher_icon.svg" alt={Sefaria._('Toggle Module Switcher')}/></a> | ||
<a className="dropdownLinks-button" onClick={handleClick}> | ||
{menuIconComponent} | ||
</a> | ||
<div className={`dropdownLinks-menu ${ isOpen ? "open" : "closed"}`}> | ||
<div className="dropdownLinks-options"> | ||
{children} | ||
|
@@ -87,6 +94,6 @@ const DropdownMenu = ({children}) => { | |
export { | ||
DropdownMenu, | ||
DropdownMenuSeparator, | ||
DropdownMenuItem, | ||
DropdownMenuItemWithIcon | ||
DropdownMenuItemWithIcon, | ||
DropdownMenuItem | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are these double slashes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To make the URL not relative (i.e. route to
sheets.sefaria.org
vslocalhost:8000/sheets.sefaria.org
). It's a bit hacky, is there a better way to do this?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be
/sheets
, confirm with SK.