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

UI: Update the toolbar button styles #16429

Merged
merged 12 commits into from
Oct 29, 2021
Merged

Conversation

MichaelArestad
Copy link
Contributor

@MichaelArestad MichaelArestad commented Oct 20, 2021

What I did

I updated the toolbar's icon button styles to match the designs @dom and I worked on recently. The new styles differentiate themselves from the tab interface and add clarity as to what state an icon button is in.

image

Here is a quick demo video:

IconButton-demo.mp4

Some details:

  • The horizontal space between icon buttons is 5px.
  • The icons have been downsized from 15px to 14px.
  • The spacers have had a margin of 8px added to match the distance between the top/bottom of an icon button and the edge of the toolbar.
  • The icons' containers have had 5px of padding to the left and right sides.
  • The hover state background color is grey, but will update to a blue to match the designs once UI: Update sidebar hover color to be a refreshing transparent blue #16408 is merged.
  • I did write a nice transition for hover states, but commented it out as I think it just feels snappier without the transition. Feel free to try it out.

How to test

  1. Open the Official Storybook using the link below.
  2. Try out various buttons in different locations including the canvas toolbar and the addon panel controls.
  3. Try keyboard navigation of the buttons. Is it clear which button the keyboard is focused on?

Before

image

After

image


  • Is this testable with Jest or Chromatic screenshots?
  • Does this need a new example in the kitchen sink apps?
  • Does this need an update to the documentation?

@nx-cloud
Copy link

nx-cloud bot commented Oct 20, 2021

Nx Cloud Report

CI ran the following commands for commit a396d5d. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

Status Command
#000000 nx run-many --target=prepare --all --parallel --max-parallel=15

Sent with 💌 from NxCloud.

@nx-cloud
Copy link

nx-cloud bot commented Oct 20, 2021

Nx Cloud Report

We didn't find any information for the current pull request with the commit 22e244f.
You might need to set the 'NX_BRANCH' environment variable in your CI pipeline.

Check the Nx Cloud Github Integration documentation for more information.


Sent with 💌 from NxCloud.

@shilman shilman added maintenance User-facing maintenance tasks and removed other labels Oct 21, 2021
@shilman shilman modified the milestones: 6.4 improvements, 6.4 PRs Oct 21, 2021
@MichaelArestad
Copy link
Contributor Author

I updated the colors, added stories, design files, and updated the screenshots. I think this is ready for review.

@yannbf
Copy link
Member

yannbf commented Oct 25, 2021

@MichaelArestad could you please test an example where a custom theme is used in manager.js? Thank you!

@MichaelArestad
Copy link
Contributor Author

Alrighty! I have good news. It looks like a theme customization as seen on the PX Blue storybook will be compatible. In fact, it would look even better.

Here is a quick demo of what my test looks like:

theme-color-demo.mp4

The eagle-eyed among you may have noticed how bad the selected item in the sidebar looks with this theme, where in PX Blue, the sidebar highlight looks good. That's because they wrote a style override targeting that element:

image

In other words, this change will not cause a regression.

To test:

  1. Update the import on line 3:
import { create, styled } from '@storybook/theming';
  1. Add some theme overrides just below PrefixIcon:
const theme = create({
  base: 'light',

  // Base colors
  colorSecondary: 'white',

  // Toolbar default and active colors
  barTextColor: '#b3d7ec',
  barSelectedColor: 'white',
  barBg: '#037ac0',
});
  1. Set the theme with the new theme constant:
addons.setConfig({
  theme: theme, // { base: 'dark', brandTitle: 'Storybook!' },
  previewTabs: {
    canvas: null,
...

@domyen If you get a chance, could you take another look and review this?

@yannbf
Copy link
Member

yannbf commented Oct 26, 2021

Alrighty! I have good news. It looks like a theme customization as seen on the PX Blue storybook will be compatible. In fact, it would look even better.

Here is a quick demo of what my test looks like:

theme-color-demo.mp4

The eagle-eyed among you may have noticed how bad the selected item in the sidebar looks with this theme, where in PX Blue, the sidebar highlight looks good. That's because they wrote a style override targeting that element:

image

In other words, this change will not cause a regression.

To test:

  1. Update the import on line 3:
import { create, styled } from '@storybook/theming';
  1. Add some theme overrides just below PrefixIcon:
const theme = create({
  base: 'light',

  // Base colors
  colorSecondary: 'white',

  // Toolbar default and active colors
  barTextColor: '#b3d7ec',
  barSelectedColor: 'white',
  barBg: '#037ac0',
});
  1. Set the theme with the new theme constant:
addons.setConfig({
  theme: theme, // { base: 'dark', brandTitle: 'Storybook!' },
  previewTabs: {
    canvas: null,
...

@domyen If you get a chance, could you take another look and review this?

Awesome stuff!! I quite like how that looks by the way!

@MichaelArestad
Copy link
Contributor Author

I updated the focus and hover styles:

updated-focus-styles.mp4

Now, it should be a little clearer when a user enables or disables a button via keyboard actions. Note: the outline only shows on Mac in some browsers when that setting is enabled in Mac's General settings.

@MichaelArestad
Copy link
Contributor Author

Just pushed an update to the Notification component so it will play nicely with the new IconButton styles. It should also be a little more resilient to changes now.

@shilman shilman changed the title Update the toolbar button styles UI: Update the toolbar button styles Oct 29, 2021
@shilman shilman merged commit bf80fea into next Oct 29, 2021
@shilman shilman deleted the update-toolbar-button-styles branch October 29, 2021 14:11
@gaetanmaisse gaetanmaisse mentioned this pull request Oct 31, 2021
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
components maintenance User-facing maintenance tasks ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants