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

Allow toolbar button background to receive hover state change #884

Merged

Conversation

hlyang397
Copy link
Contributor

@hlyang397 hlyang397 commented Nov 12, 2020

Issue

When mouseover to toolbar button background (not the icon), the toolbar button background won't change the color.
It's my bad that I didn't fix it 100% correctly in #858.

Updates

  • allow toolbar button background color to receive hover state change
  • use fill:transparent instead of fill: none

Demo screenshot or recording

Before fix:
before_fix

After fix:
after_fix

Review checklist (for reviewers only)

  • Demos all features
  • Documented/annotated
  • Matches UI/UX specs
  • Meets the code style guide
  • Accessible
  • Mobile first (responsive)
  • RTL support (bidirectional text)
  • Performant (limited bloat)

- allow hover event to be triggered
@hlyang397 hlyang397 requested review from zvonimirfras and removed request for a team November 12, 2020 05:34
@hlyang397 hlyang397 changed the title fix: allow toolbar button background to receive hover state change Allow toolbar button background to receive hover state change Nov 12, 2020
Copy link
Member

@theiliad theiliad left a comment

Choose a reason for hiding this comment

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

There's a bigger issue here, most zoombar demos are missing the toolbar

image

@hlyang397
Copy link
Contributor Author

There's a bigger issue here, most zoombar demos are missing the toolbar

@theiliad

We had all these zoom bar demos before we got toolbar PR merged, so that is expected.
Do you like to enable toolbar in zoom bar demos in this PR?

@hlyang397 hlyang397 requested a review from theiliad November 13, 2020 04:14
@theiliad theiliad merged commit 681ec23 into carbon-design-system:master Nov 16, 2020
@theiliad theiliad deleted the fix-toolbar-button-bg branch November 16, 2020 15:40
JennChao added a commit to JennChao/carbon-charts that referenced this pull request Nov 17, 2020
…on-charts into sparkline-with-scale-color-option

* 'master' of https://github.com/carbon-design-system/carbon-charts: (24 commits)
  v0.41.4
  Merge pull request carbon-design-system#882 from theiliad/demo-treemap-fix
  docs(core): add theme & API tutorials to storybook (carbon-design-system#883)
  v0.41.3
  Merge pull request carbon-design-system#880 from sophiiae/fix-import-colors
  fix(core): use fill:transparent for toolbar controls (carbon-design-system#884)
  v0.41.2
  fix(core): tooltips should hide when mouseout is triggered on the chart holder (carbon-design-system#887)
  fix(core): sanitize user provided axis domains
  add treemap to the component status table
  v0.41.1
  Merge pull request carbon-design-system#719 from theiliad/tree-map
  fix(core): correct axis title paddings in y-axes (carbon-design-system#873)
  v0.41.0
  fix: use fill:none to allow transparent toolbar control background (carbon-design-system#858)
  feat(core): add alignment options for gauge charts (carbon-design-system#877)
  v0.40.14
  fix(core): replace import to lodash with lodash-es
  fix(core): only set zoombar data and sanitize date values if a cartesian time-series chart
  v0.40.13
  ...
theiliad pushed a commit to theiliad/carbon-charts that referenced this pull request May 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants