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

Fix item pane header #4159

Merged
merged 14 commits into from
Jun 4, 2024
Merged

Conversation

windingwind
Copy link
Contributor

@windingwind windingwind commented May 23, 2024

fix: #4141
Move split-menu-button styles to SCSS

  • There is currently almost no spacing between the title area and the buttons. Should be 4px in tight, and 8px in comfortable
  • Decrease the margins around and between the buttons to 8px
  • Improve the paddings of the two buttons. They are meant to look like system native buttons, but right now they look slightly off on both mac and windows. The padding before the icon in "Add to My Library" is especially large
  • Are the quotation marks around "My Library" necessary? I think we can remove those
  • We should use accent-blue for the library icon
  • The color of the divider and the chevron are both wrong in dark mode

@windingwind windingwind changed the title Fix-item-pane-header Fix item pane header May 23, 2024
@windingwind
Copy link
Contributor Author

fix: #4116

@windingwind windingwind marked this pull request as ready for review May 29, 2024 12:14
@windingwind
Copy link
Contributor Author

Hi @yexingsha, this is what this PR looks like, please check if I missed anything:

MacOS:

image image

Windows:
image
image

@yexingsha
Copy link
Contributor

On macOS, the text and icon are positioned 1px too high within the button (it's more obvious with the library icon). And both the spacing and padding are still a bit too large. Seems like the extent of the button is about 2px wider than its optical width, so need to compensate for that:

Screenshot 2024-05-29 at 17 25 44

The spacing between the icon and text can be reduced to 4px.

The header looks much better now at narrow width, but looks weird at larger width:

Screenshot 2024-05-29 at 16 31 51

I think once the "Add to" button grows wide enough to fully display the text, both buttons should grow in width as the pane gets wider. And perhaps the split menu button should still align to center, even though it does go against convention.

@windingwind
Copy link
Contributor Author

Thanks. Done:

  • the text and icon are positioned 1px too high within the button
  • And both the spacing and padding are still a bit too large. Seems like the extent of the button is about 2px wider than its optical width, so need to compensate for that
  • The spacing between the icon and text can be reduced to 4px.
  • And perhaps the split menu button should still align to center, even though it does go against convention.
  • I think once the "Add to" button grows wide enough to fully display the text, both buttons should grow in width as the pane gets wider
Screen.Recording.2024-05-31.at.08.48.16.mov

@yexingsha
Copy link
Contributor

This looks great!

@dstillman dstillman merged commit a6076ce into zotero:main Jun 4, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants