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

Accordion - experimental component (feedback from review) #1671

Closed
6 tasks done
laurenmrice opened this issue Jan 21, 2019 · 5 comments
Closed
6 tasks done

Accordion - experimental component (feedback from review) #1671

laurenmrice opened this issue Jan 21, 2019 · 5 comments
Assignees
Labels
role: dev 🤖 type: bug 🐛 version: 10 Issues pertaining to Carbon v10
Milestone

Comments

@laurenmrice
Copy link
Member

laurenmrice commented Jan 21, 2019

  • hover needs to completely go over the rule lines
  • focus needs to be on the whole row, not the chevron (safari doesnt have focus indicator)
  • motion updates
  • row height is 35px and should be 32px
  • line height needs to be 20 not 21px
  • fix content padding for mobile view to take up full width up until 16px from svg instead of 75%
asudoh added a commit to asudoh/carbon-components that referenced this issue Jan 22, 2019
@asudoh
Copy link
Contributor

asudoh commented Jan 22, 2019

Got quick fix for some, @laurenmrice the light height seems font size (14px) times 1.5, do we want it non-relative to the font size? Also wondering what you meant by the first item. Thanks!

@laurenmrice
Copy link
Member Author

laurenmrice commented Jan 22, 2019

@asudoh Yes the type size is 14px but the line height of spacing should be set at 20px (1.25 rem). The type token for this is body-long-01.

For the first item about hover, currently when you hover over rows, the lines between the rows are showing up ontop of the hover. We want to cover these lines on the row when the hover is activated

screen shot 2019-01-22 at 10 00 04 am

@asudoh
Copy link
Contributor

asudoh commented Jan 23, 2019

Thanks @laurenmrice for clarifying! Addressed the latter. The former seems to depend on #1546.

asudoh added a commit that referenced this issue Jan 23, 2019
* fix(accordion): minor UI changes

Refs #1671.

* fix(accordion): cover up border with hover background
@asudoh
Copy link
Contributor

asudoh commented Jan 24, 2019

From @laurenmrice (#1674 (review)):

Looks good. But one thing I did notice on the focus states is that the blue outline is within the two line dividers of each row. When the focus is activated, the lines for that row should not be visible.

screen shot 2019-01-24 at 9 46 55 am

asudoh added a commit to asudoh/carbon-components that referenced this issue Jan 25, 2019
asudoh added a commit that referenced this issue Jan 25, 2019
@jnm2377 jnm2377 self-assigned this Feb 18, 2019
@asudoh asudoh added this to the v10.0-rc1 milestone Feb 19, 2019
@carbon-bot
Copy link
Contributor

🎉 This issue has been resolved in version 9.80.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
role: dev 🤖 type: bug 🐛 version: 10 Issues pertaining to Carbon v10
Projects
None yet
Development

No branches or pull requests

4 participants