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

Chore/Vertically center text labels in MuiDrawer side menu #2954

Conversation

knakamura13
Copy link
Contributor

Summary

This pull request addresses vertical alignment issues in the side menu (MuiDrawer component) by making the following improvements:

  1. Vertical Alignment of Side Menu Button Labels

    • Fixed the vertical alignment of labels, such as 'Agentflows', which were not centered within their containers.
    • Applied automatic vertical margins to <ListItemText> and added a 4px vertical margin to the primary text element <Typography> to maintain existing margins.
  2. Vertical Alignment of Side Menu BETA Chips

    • Corrected the vertical alignment of 'BETA' chips in the side menu.
    • Ensured that the chips are consistently centered regardless of the presence of captions in the corresponding text labels.
  3. Adjust Margin for Secondary Labels (a.k.a., captions)

    • Added mt: -0.6 to the secondary (caption) label to replicate the original gap between the primary and secondary labels.

Before and After Screenshots

Before After
Before After

Before and After Screenshots with Added Caption Labels

Before with Caption After with Caption
Before with caption After with caption

Detailed Changes

  • Updated <ListItemText> components with sx={{ my: 'auto' }} to automatically center the nav button labels vertically.
  • Modified <Typography> elements within <ListItemText> to include sx={{ my: 0.5 }} for consistent vertical margins, replicating the 4px vertical margins that existed before this change.
  • Updated 'BETA' chip elements with sx={{ my: 'auto' }} to automatically center them vertically.
  • Added mt: -0.6 to the secondary (caption) label to replicate the original gap between the primary and secondary labels.

Additional Information

These changes enhance the visual alignment and consistency of the side menu items, improving the overall user experience.

- Some labels, such as 'Agentflows', were not vertically centered in
  their containers in the side menu (MuiDrawer component).
- Fixed the alignment by styling text containers (`<ListItemText>`) with
  automatic vertical margins.
- Maintained existing vertical margins by adding a 4px vertical margin
  to the primary text element (`<Typography>`).
- Note: future work is needed to fix vertical alignment of the BETA
  chip, plus the margin between primary text elements and the secondary
  captions should probably be reduced by half.
- The 'BETA' chips in the side menu were not vertically centered in
  their containers in the side menu (MuiDrawer component).
- These chips previously appeared to be vertically centered only by
  coincidence and only when their corresponding text label had no caption.
- Added `mt: -0.6` to the secondary (caption) label to replicate the
  original gap between the primary and secondary labels.
- This adjustment was needed only after the vertical alignment of the
  primary text labels were fixed in a prior commit.
@toi500
Copy link
Contributor

toi500 commented Aug 7, 2024

Not sure what three-word caption would fit in there as useful information for the user, but if this change goes ahead, I would prefer captions to appear on hover (or select) for a minimalist approach.

@knakamura13
Copy link
Contributor Author

knakamura13 commented Aug 7, 2024

Not sure what three-word caption would fit in there as a useful information for the user, but if this change goes ahead, I would prefer captions to appear on hover for a minimalist approach.

Captions were already a feature (I didn't just come up with that feature on my own), but none of the menu items had an active caption. I wanted to ensure that, if captions are ever enabled in the future, the design with my changes looks good.

@toi500
Copy link
Contributor

toi500 commented Aug 7, 2024

@knakamura13 I see, that makes more sense.

A change I would pay for is to add *{ border: 0; } as an optional feature for the entire app. It would look sleek.

There is still some room for visual improvements in this area, IMHO.

Thank you for your work on this.

Copy link
Contributor

@HenryHengZJ HenryHengZJ left a comment

Choose a reason for hiding this comment

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

thank you @knakamura13 !

@HenryHengZJ HenryHengZJ merged commit 44adebe into FlowiseAI:main Aug 7, 2024
2 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.

3 participants