Skip to content

(LIDAR-555) Add neutral tag #339

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
Merged
merged 2 commits into from
Nov 3, 2020
Merged

(LIDAR-555) Add neutral tag #339

merged 2 commits into from
Nov 3, 2020

Conversation

Lukeaber
Copy link
Collaborator

The neutral tag component as off designs https://www.figma.com/file/9RzmjE0XYWWADWLKOJP5mR/PDS-UI-Components?node-id=2219%3A3301 .

A few assumptions that have been made:

  • That is was okay to add a iconSize prop to the button component
  • That the tag 'type' prop was the correct prop to use, i know theres talk of changing this prop on the buttons etc. but wasn't sure if that applied here

@Lukeaber Lukeaber requested a review from a team as a code owner October 14, 2020 11:08
@Lukeaber
Copy link
Collaborator Author

Screenshot 2020-10-14 at 12 15 29

@vine77
Copy link
Contributor

vine77 commented Oct 14, 2020

@Sigler What should the props be named here?

@vine77 vine77 mentioned this pull request Oct 14, 2020
Closed
1 task
@Lukeaber Lukeaber force-pushed the PDS-555-Add-neutral-tag branch from 0dff486 to 65ab0af Compare October 15, 2020 14:14
@vine77
Copy link
Contributor

vine77 commented Oct 22, 2020

@Sigler has a matrix for the future of prop naming. It looks like type is intended to be reserved for color, so this would be more like emphasis="subtle".

image

@vine77
Copy link
Contributor

vine77 commented Oct 28, 2020 •
edited
Loading

Let's make this new variant type="neutral" and emphasis="subtle", and make the "X" button grey instead of blue to future-proof it.

vine77
vine77 suggested changes Oct 28, 2020
View reviewed changes
Copy link
Contributor

@vine77 vine77 left a comment

Choose a reason for hiding this comment

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

See comments

@vine77 vine77 self-assigned this Oct 28, 2020
@Lukeaber
Copy link
Collaborator Author

Lukeaber commented Nov 2, 2020

Screenshot 2020-11-02 at 13 58 10

@Lukeaber Lukeaber force-pushed the PDS-555-Add-neutral-tag branch from 90614e7 to 4996deb Compare November 2, 2020 14:02
vine77
vine77 approved these changes Nov 3, 2020
View reviewed changes
Copy link
Contributor

@vine77 vine77 left a comment

Choose a reason for hiding this comment

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

LGTM!

@vine77 vine77 merged commit 9e0c999 into development Nov 3, 2020
@vine77 vine77 deleted the PDS-555-Add-neutral-tag branch November 3, 2020 06:34
@vine77 vine77 mentioned this pull request Nov 3, 2020
Merged
@vine77 vine77 mentioned this pull request Nov 12, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vine77 vine77 vine77 approved these changes

Assignees

@vine77 vine77

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Lukeaber @vine77