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

feat: restructure menubar #2 #6411

Merged
merged 5 commits into from
Sep 25, 2024
Merged

feat: restructure menubar #2 #6411

merged 5 commits into from
Sep 25, 2024

Conversation

mejo-
Copy link
Member

@mejo- mejo- commented Sep 16, 2024

📝 Summary

  • refactor(menu): Support several types in a list for isActive check
  • feat(menu): Group block elements in a submenu
  • fix(menu): Move heading left of bold, links and attachments neighbors
  • fix(menu): change headings submenu icon to FormatSize icon
  • Contributes to: Menu bar redesign #2836

🖼️ Screenshots

🏚️ Headings menu before 🏡 Headings menu after
image image
🏚️ Callouts menu before 🏡 Blocks menu after
image image
Screencast
recording1

🏁 Checklist

  • Code is properly formatted (npm run lint / npm run stylelint / composer run cs:check)
  • Sign-off message is added to all commits
  • Tests (unit, integration and/or end-to-end) passing and the changes are covered with tests

Copy link
Member

@marcoambrosini marcoambrosini left a comment

Choose a reason for hiding this comment

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

hmm not sure about the block icon, I think it's not too clear 🤔

@juliusknorr
Copy link
Member

I find it quite fitting, also considering the other icons we looked at this week

@mejo-
Copy link
Member Author

mejo- commented Sep 17, 2024

hmm not sure about the block icon, I think it's not too clear 🤔

@marcoambrosini I find it fitting well, given that it shows a box with text inside - which describes text boxes pretty well. Do you have an idea for an alternative icon? And are you against the icon or just not 100% sure?

@mejo- mejo- added 3. to review enhancement New feature or request design Experience, interaction, interface, … and removed 2. developing labels Sep 17, 2024
@max-nextcloud
Copy link
Collaborator

I like it. I'm not sure about the 'no heading' icon with the two T though. If the text is not bold the icon is just deselected and does not change to a thin 'F' either. Same for the lists. If no list type is selected the icon still displays a list and not a 'non-list' state.

@mejo-
Copy link
Member Author

mejo- commented Sep 18, 2024

I'm not sure about the 'no heading' icon with the two T though. If the text is not bold the icon is just deselected and does not change to a thin 'F' either. Same for the lists. If no list type is selected the icon still displays a list and not a 'non-list' state.

I implemented it as it was once decided in #2836. I think the idea was that it's a more generic icon to communicate "text size" than "H1". I don't have a strong opinion on which one communicates better. "H1" might also be hard to understand for non-english speakers. Maybe designers can comment here @marcoambrosini @nimishavijay 😊

@marcoambrosini
Copy link
Member

I find it:

  • too similar to lists > difficult to quickly find
  • Rectangle shape similar to neighbouring icons > difficult to quickly find
  • reminds me of left-alignment icon > possibly confusing

Maybe brackets are good for this case?

Screenshot 2024-09-19 at 09 17 58

So I would go with:
https://fonts.google.com/icons?selected=Material+Symbols+Outlined:data_array:FILL@0;wght@400;GRAD@0;opsz@24&icon.query=parenthe&icon.size=24&icon.color=%235f6368

@juliusknorr
Copy link
Member

Brackets I would find to easy to only associate with a code block, not sure this is a good one for average users as well to refer to as blocks

@mejo-
Copy link
Member Author

mejo- commented Sep 24, 2024

I tried the brackets and have to say that both work well in my opinion. I don't have a strong opinion here. Maybe @nextcloud/designers can give a final word which one to use?

🖼️ CardText 🖼️ CodeBrackets
image image

@nimishavijay
Copy link
Member

My vote would go for the second one :) it looks more like a generic box

@mejo-
Copy link
Member Author

mejo- commented Sep 24, 2024

My vote would go for the second one :) it looks more like a generic box

Agreed and implemented 😊

@mejo-
Copy link
Member Author

mejo- commented Sep 24, 2024

I like it. I'm not sure about the 'no heading' icon with the two T though. If the text is not bold the icon is just deselected and does not change to a thin 'F' either. Same for the lists. If no list type is selected the icon still displays a list and not a 'non-list' state.

I implemented it as it was once decided in #2836. I think the idea was that it's a more generic icon to communicate "text size" than "H1". I don't have a strong opinion on which one communicates better. "H1" might also be hard to understand for non-english speakers. Maybe designers can comment here @marcoambrosini @nimishavijay 😊

@nimishavijay what's your take on this?

@nimishavijay
Copy link
Member

I think the idea was that it's a more generic icon to communicate "text size" than "H1". I don't have a strong opinion on which one communicates better. "H1" might also be hard to understand for non-english speakers.

I agree :) H1, H2, etc are almost rather technical markdown terms, and it's really just the size of the heading, so I would go for the T icon :)

@mejo- mejo- requested a review from elzody September 25, 2024 10:13
@mejo- mejo- merged commit 190cc2c into main Sep 25, 2024
59 of 61 checks passed
@mejo- mejo- deleted the feat/menubar_block_submenu branch September 25, 2024 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review design Experience, interaction, interface, … enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants