added arrow to a11y addon HeaderBar

[jmiazga]

Issue: #3641

What I did

Added arrow to indicate button is expandable. Rotate arrow when a11y violation is expanded.

Closed:

Expanded:

How to test

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?

If your answer is yes to any of these, please make sure to include it in your PR.

For maintainers only: Please tag your pull request with at least one of the following:
["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]

[Keraito]

I totally forgot about this one, thanks for picking it up!

[jmiazga]

@Keraito It looks like a build step failed, I don't think the problem was introduced by me. Can you check it out?

[Hypnosphi]

Let's use the same symbol as in sidebar stories list: https://github.com/storybooks/storybook/blob/master/lib/ui/src/modules/ui/components/stories_panel/stories_tree/tree_decorators.js#L18

[Keraito]

Hey @jmiazga, I don't think you have to worry about that for now. I agree with the above comment that it's better to use the svg component that already exists in the project. That was also the direction that I took when I was looking into the issue, but the main difficulty is getting the proper properties for it. Would you like to give this approach a shot?

[jmiazga]

@Hypnosphi @Keraito I'll take a look at using whats in react-icons

[Keraito]

@jmiazga I think the easiest is to use the IoChevronRight that was linked to, which is the same as currently used in the sidebar stories list. But if you can find something better due to specific reasons, go ahead.

[jmiazga]

@Keraito yep thats the plan

[jmiazga]

@Keraito I can use IoChevronRight and rotate it with css, or swap it with IoChevronDown, do you have a preference?

[codecov]

Codecov Report

Merging #3788 into master will decrease coverage by <.01%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #3788      +/-   ##
==========================================
- Coverage   41.56%   41.56%   -0.01%     
==========================================
  Files         455      455              
  Lines        5177     5178       +1     
  Branches      899      899              
==========================================
  Hits         2152     2152              
- Misses       2485     2486       +1     
  Partials      540      540

Impacted Files	Coverage Δ
addons/a11y/src/components/Report/Item.js	0% <0%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0e565c0...733ab71. Read the comment docs.

[jmiazga]

Here are some updated screen grabs:

[jmiazga]

@Keraito any feedback on this?

[Hypnosphi]

I'd say 1px higher in collapsed state

[jmiazga]

@Hypnosphi Made the change, here are some updated screen grabs

[igor-dv]

I think should wait for the #3628 merging, it much easier to solve conflicts here.

[jmiazga]

@igor-dv Ill keep an eye on #3628 and update from master once it's merged in.

[Hypnosphi]

How do you feel about adding some CSS transition?

[jmiazga]

@Hypnosphi I can add something. Is there a similar transition somewhere you'd like to replicate?

[Hypnosphi]

Something like transition: transform 0.3s ease-in-out; should work

I also don't really like how it jumps when expanding because of appearing scrollbar (on mac with touchpad you can observe that when enabling "Show scrollbars: always" in system settings -> general). Maybe we shouldn't use text-align: center there?

[igor-dv]

FYI, theming PR was merged.

[ndelangen]

@jmiazga Can I help you get this PR in a merge-able state?

[ndelangen]

Look like the cli failed because of an npm failure, I will rerun the task

[jmiazga]

@Hypnosphi I haven't had time to address the transition for the content when expanded or the scrollbar. The transition for content can't be solved by css alone because the elements aren't always in the dom. I need to check out how it's being done in the menu on the left side.

The arrow is there and it has a transition, so this could always be merged and I can follow up with another PR to address the two issues mentioned above.

[danielduan]

thanks for the PR. when you get a chance, please follow up with another PR like you've mentioned.

[ndelangen]

@jmiazga There are some more improvements possible to the a11y addon, don't you think?

Are you on the storybook slack? If you'd like we could discuss them there?
https://now-examples-slackin-rrirkqohko.now.sh/