-
Notifications
You must be signed in to change notification settings - Fork 10
[T2898] FIX: user menu component #249
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
[T2898] FIX: user menu component #249
Conversation
# FEAT: added optional y-padding to buttons # FEAT: new 'Login' variant for buttons # FEAT: new circular option for buttons
Summary of ChangesHello @Danielgergely, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on enhancing the styling capabilities of button components within the theme. It introduces greater flexibility for vertical padding through updated SCSS mixins and adds new button styles, specifically a 'login' variant and a 'circular' radius option. These new styles are then applied to the user menu component, refining its visual presentation and ensuring consistency with the new design options. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
The pull request introduces new button styling capabilities, specifically optional vertical padding, a new 'Login' button variant, and a 'circular' radius option. These new styles are then applied to the user menu component in the header, changing its appearance from a compact pill-shaped button to a login-specific circular button. The changes are well-implemented and consistent across the SCSS and XML files, enhancing the flexibility and visual options for buttons within the theme.
NoeBerdoz
left a comment
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.
Thanks for your PR!
Here is a thing to consider and a suggestion:
Did you test the behavior of the button when it has your implemented style and a label inside ?
this is how it is attended to be:

Then, i would suggest to rename &--login-hollow into something more general as it's a reusable theme attribute, for example something like &--slim-hollow
# STYLE: Changed the restrictive 'login' class name to a more general one: 'slim' # FIX: Conditional styling of the slim/compact sign-in button
NoeBerdoz
left a comment
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.
Nice, thank you !
FEAT: added optional y-padding to buttons
FEAT: new 'Login' variant for buttons
FEAT: new circular option for buttons