Skip to content
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

User Dropdown for Logged out Library #2000

Conversation

saengel
Copy link
Contributor

@saengel saengel commented Aug 18, 2024

Description

Dropdown menu for modularized library, specifically for logged out users.

Code Changes

The following changes were made to the files below:

  1. static/css/s2.css - Some minor padding shifts, adding styling for the language toggle with a pseudo-class for the dot separator.
  2. static/icons/logged_out.svg - The new logged out icon
  3. static/js/common/DropdownMenu.jsx - Improving the generic <DropdownMenu /> component to enhance reusability (i.e. making the icon for the dropdown customizable, and control over whether or not to open links in new tabs).
  4. static/js/Header.jsx - Adding the new logged out component (<LoggedOutMenu />), implementing the shifts to the generic component mentioned above in the module switcher.

@saengel saengel self-assigned this Aug 18, 2024
@saengel saengel requested a review from edamboritz August 18, 2024 17:25
padding: 10px 2px;
}

.englishLanguageButton::after {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can actually make this a bit more general, not needing to create a special class for the enlish link

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good thinking!

I did this because of annoying spacing issues with r/l padding on English vs Hebrew buttons. Is that OK in this case? Happy to show you if it's clearer that way :)

@saengel saengel merged commit 76632ba into modularization-main Sep 25, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants