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.

Sign up for GitHub

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

Jump to bottom

fix(progressindicator): adjust styles for progress step with optional label position #5356

Merged
merged 8 commits into from
Feb 17, 2020
Merged

fix(progressindicator): adjust styles for progress step with optional label position #5356

merged 8 commits into from
Feb 17, 2020

Conversation

abbeyhrt
Copy link
Contributor

@abbeyhrt abbeyhrt commented Feb 14, 2020 •
edited
Loading

This PR adjusts how the ProgressSteps with an optional label position themselves and resolves a release bug that was caught before the 10.10 release.

What the bug was:

Screen Shot 2020-02-14 at 10 44 01 AM

Changelog

Changed

  • switches vertical progress bar to display flex

Removed

  • initial position from optional label

Testing / Reviewing

Check to make sure the icons are all in line

@abbeyhrt abbeyhrt requested a review from a team as a code owner February 14, 2020 17:37
@abbeyhrt abbeyhrt requested review from a team and jeanservaas and removed request for a team February 14, 2020 17:37
@ghost ghost requested review from emyarod and joshblack February 14, 2020 17:37
@abbeyhrt abbeyhrt requested review from laurenmrice and removed request for jeanservaas February 14, 2020 17:43
@netlify
Copy link

netlify bot commented Feb 14, 2020

Deploy preview for carbon-elements ready!

Built with commit 0acf4bf

https://deploy-preview-5356--carbon-elements.netlify.com

@netlify
Copy link

netlify bot commented Feb 14, 2020

Deploy preview for carbon-components-react ready!

Built with commit 0acf4bf

https://deploy-preview-5356--carbon-components-react.netlify.com

@netlify
Copy link

netlify bot commented Feb 14, 2020 •
edited
Loading

Deploy preview for carbon-components-react ready!

Built with commit ca8afd0

https://deploy-preview-5356--carbon-components-react.netlify.com

@netlify
Copy link

netlify bot commented Feb 14, 2020 •
edited
Loading

Deploy preview for carbon-elements ready!

Built with commit ca8afd0

https://deploy-preview-5356--carbon-elements.netlify.com

laurenmrice
laurenmrice requested changes Feb 14, 2020
View reviewed changes
Copy link
Member

@laurenmrice laurenmrice left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Can someone view the vertical progress indicator in ie11 or edge? I think that is where this issue was occurring. Positioning looks good in firefox, chrome and safari.

Now that I have access to view themes in react:

  • the current and completed icons needs to be calling interactive-04 they are wrong in the dark themes.
  • the fourth and fifth step labels should be calling text-01. They are wrong in the dark themes.
  • the optional text should be calling text-02

@abbeyhrt
Copy link
Contributor Author

@laurenmrice surprisingly, this problem was in all browsers, but someone should definitely still check IE11!

emyarod
emyarod approved these changes Feb 14, 2020
View reviewed changes
Copy link
Member

@emyarod emyarod left a comment

Choose a reason for hiding this comment

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

Edge screenshot for reference

image

looks good to me pending design approval

laurenmrice
laurenmrice approved these changes Feb 14, 2020
View reviewed changes
Copy link
Member

@laurenmrice laurenmrice left a comment

Choose a reason for hiding this comment

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

All looks good to me in firefox, chrome and safari and in diff themes !

tw15egan
tw15egan approved these changes Feb 17, 2020
View reviewed changes
Copy link
Collaborator

@tw15egan tw15egan left a comment

Choose a reason for hiding this comment

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

👍 ✅

@abbeyhrt abbeyhrt changed the title fix(progressindicator): adjust progressstep with optional label position fix(progressindicator): adjust styles for progress step with optional label position Feb 17, 2020
@abbeyhrt abbeyhrt merged commit ea14f98 into carbon-design-system:master Feb 17, 2020
This was referenced Feb 19, 2020
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@laurenmrice laurenmrice laurenmrice approved these changes

@tw15egan tw15egan tw15egan approved these changes

@emyarod emyarod emyarod approved these changes

@joshblack joshblack Awaiting requested review from joshblack

Assignees
No one assigned
Labels
status: ready to merge 🎉
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@abbeyhrt @emyarod @tw15egan @laurenmrice