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

[core] feat(MultiStepDialog): improve step styling #4630

Merged
merged 8 commits into from
Apr 15, 2021
Merged

[core] feat(MultiStepDialog): improve step styling #4630

merged 8 commits into from
Apr 15, 2021

Conversation

kmblake
Copy link
Contributor

@kmblake kmblake commented Apr 12, 2021 •
edited
Loading

Fixes #0000

Checklist

  • Includes tests
  • Update documentation

Changes proposed in this pull request:

  • Only make currently active step blue.
  • Adjust right to left panel ratios to match original mocks.
  • Set min-width on right panel to prevent overflow
  • Make step title flex: 1 to allow clients to provide a title + right-aligned icon for a step.

Reviewers should focus on:

Screenshot

Before this PR:
image

After this PR:
image

@kmblake kmblake marked this pull request as draft April 12, 2021 14:47
@kmblake kmblake marked this pull request as ready for review April 12, 2021 15:34
@kmblake kmblake requested a review from adidahiya April 12, 2021 15:34
@kmblake
Copy link
Contributor Author

kmblake commented Apr 14, 2021

Also adjusted step colors slightly per design feedback:

"Seen" step title is in the Default text colour
Dark mode matches so "Seen" step title is also in the Default dark mode colour (#F5F8FA, 100%)
I had "Unseen" step title as Muted text before, but I think it makes more sense to use Disabled Default text so it translates to dark mode easier. So for light mode (#5C7080 60%;) and dark mode (#A7B6C2 60%;)

@adidahiya adidahiya self-assigned this Apr 14, 2021
adidahiya
adidahiya suggested changes Apr 15, 2021
View reviewed changes
Copy link
Contributor

@adidahiya adidahiya left a comment

Choose a reason for hiding this comment

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

looks good aside from these minor comments

packages/core/src/components/dialog/_multistep-dialog.scss Outdated Show resolved Hide resolved
packages/core/src/components/dialog/_multistep-dialog.scss Outdated Show resolved Hide resolved
@adidahiya adidahiya merged commit 31d4bf8 into palantir:develop Apr 15, 2021
@kmblake kmblake mentioned this pull request Apr 16, 2021
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@invliD invliD invliD left review comments

@adidahiya adidahiya adidahiya approved these changes

Assignees

@adidahiya adidahiya

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kmblake @invliD @adidahiya