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(button): ensure loading icon does not make button taller #395

Merged
merged 4 commits into from
Feb 25, 2020

Conversation

jendowns
Copy link
Contributor

@jendowns jendowns commented Feb 21, 2020

Affected issues

Proposed changes

  • add negative top/button margins to the loading animation icon (I think this is the simplest way to handle the issue where the loading icon makes the button taller -- because the icon itself is so tall 🤔 I experimented with absolute positioning, setting strict heights, etc., but this works cleanly in the latest Firefox, Safari, and Chrome. Definitely let me know what you think though!)

Testing instructions

  • toggle the Loading knob in storybook and then check the button heights for default, small, field, and large. The Loading variation should be the same height as the non-loading variations.

@jendowns jendowns requested a review from a team as a code owner February 21, 2020 17:36
@jendowns jendowns requested a review from SimonFinney February 21, 2020 17:36
@netlify
Copy link

netlify bot commented Feb 21, 2020

Deploy preview for ibm-security ready!

Built with commit a6c36bb

https://deploy-preview-395--ibm-security.netlify.com

@netlify
Copy link

netlify bot commented Feb 21, 2020

Deploy preview for ibm-security ready!

Built with commit 973d1b7

https://deploy-preview-395--ibm-security.netlify.com

Copy link
Contributor

@SimonFinney SimonFinney left a comment

Choose a reason for hiding this comment

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

I've tested this at all sizes and it LGTM! 👏 Adding @carbon-design-system/ibm-security-design for design verification

@SimonFinney SimonFinney requested a review from a team February 24, 2020 11:50
Copy link

@petervachon petervachon left a comment

Choose a reason for hiding this comment

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

LGTM!

@jendowns jendowns merged commit 32f8992 into dev Feb 25, 2020
SimonFinney pushed a commit that referenced this pull request Feb 25, 2020
## [1.17.2-prerelease.5](v1.17.2-prerelease.4...v1.17.2-prerelease.5) (2020-02-25)

### Bug Fixes

* **button:** ensure loading icon does not make button taller ([#395](#395)) ([32f8992](32f8992))
@SimonFinney SimonFinney deleted the button-loading-icon branch February 26, 2020 10:11
SimonFinney pushed a commit that referenced this pull request Mar 3, 2020
# [1.18.0](v1.17.1...v1.18.0) (2020-03-03)

### Bug Fixes

* **button:** ensure loading icon does not make button taller ([#395](#395)) ([32f8992](32f8992))
* **error-page:** update error page tests and remove snapshots ([#358](#358)) ([ebe7b47](ebe7b47))
* **external-link:** add a11y and attributes tests, remove snapshots ([#365](#365)) ([fec99c1](fec99c1))
* **filter-panel:** accordion item themed gradient color ([#394](#394)) ([059d5cf](059d5cf))
* **filter-panel-accordion-item:** use outline and transparent border to prevent hover + focus overlap ([#378](#378)) ([5edb4b1](5edb4b1))
* **ica:** update tests for ica and skeleton variation ([#379](#379)) ([f9969a3](f9969a3))
* **interactive-tag:** update tests and remove old snapshots ([#382](#382)) ([f84fd45](f84fd45))
* **navitem:** prevent flex shrink of external link icon, update story demo ([#409](#409)) ([78e0579](78e0579))
* **type-layout:** flatten styles and do not clone cells ([#408](#408)) ([55fde34](55fde34))
* **type-layout:** update tests and remove old snapshots ([#383](#383)) ([0b99110](0b99110))
* **UNSTABLE__pagination:** add a11y and unit tests ([#327](#327)) ([9490e4f](9490e4f))

### Features

* **card-skeleton:** allow custom classname and extra props via spread attr ([#405](#405)) ([480a9ed](480a9ed))
@SimonFinney
Copy link
Contributor

🎉 This PR is included in version 1.18.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

SimonFinney pushed a commit that referenced this pull request Mar 12, 2020
# [2.4.0](v2.3.0...v2.4.0) (2020-03-12)

### Bug Fixes

* **button:** ensure loading icon does not make button taller ([#395](#395)) ([32f8992](32f8992))
* **button:** update testing structure, props docs, and add a11y test ([#362](#362)) ([0ee20bf](0ee20bf))
* **checkbox-skeleton:** correctly export skeleton component ([#422](#422)) ([911e8f8](911e8f8))
* **error-page:** update error page tests and remove snapshots ([#358](#358)) ([ebe7b47](ebe7b47))
* **external-link:** add a11y and attributes tests, remove snapshots ([#365](#365)) ([fec99c1](fec99c1))
* **filter-panel:** accordion item themed gradient color ([#394](#394)) ([059d5cf](059d5cf))
* **filter-panel:** add a11y tests to subcomponents, use test keyword ([#375](#375)) ([a04cb8f](a04cb8f))
* **filter-panel-accordion-item:** use outline and transparent border to prevent hover + focus overlap ([#378](#378)) ([5edb4b1](5edb4b1))
* **ica:** update tests for ica and skeleton variation ([#379](#379)) ([f9969a3](f9969a3))
* **interactive-tag:** update tests and remove old snapshots ([#382](#382)) ([f84fd45](f84fd45))
* **navitem:** prevent flex shrink of external link icon, update story demo ([#409](#409)) ([78e0579](78e0579))
* **string-formatter:** update tests ([#413](#413)) ([e5edb22](e5edb22))
* **summary-card:** add a11y and unit tests ([#337](#337)) ([78ceb90](78ceb90))
* **summary-card-action:** remove overflow hidden so tooltip can be shown ([#420](#420)) ([fb72d41](fb72d41))
* **trending-card:** reformat tests and remove snapshots ([#346](#346)) ([423f352](423f352))
* **type-layout:** flatten styles and do not clone cells ([#408](#408)) ([55fde34](55fde34))
* **type-layout:** update tests and remove old snapshots ([#383](#383)) ([0b99110](0b99110))
* **UNSTABLE__pagination:** add a11y and unit tests ([#327](#327)) ([9490e4f](9490e4f))

### Features

* **card-skeleton:** allow custom classname and extra props via spread attr ([#405](#405)) ([480a9ed](480a9ed))
@SimonFinney
Copy link
Contributor

🎉 This PR is included in version 2.4.0 🎉

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Button height changes when toggling loading prop
3 participants