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

3666 - Update the design of widget counts in uplift #4054

Merged
merged 8 commits into from
Jun 24, 2020

Conversation

ericangeles
Copy link
Contributor

@ericangeles ericangeles commented Jun 18, 2020

Explain the details for making this change. What existing problem does the pull request solve?
Implementing the new design for widget count in uplift. I've also tweaked some positions in non uplift.

This is not limiting to the four colors that was in the design, we can use all the color palettes that we have, even adding other colors in the future.

Related github/jira issue (required):
Closes #3666

Steps necessary to review your pull request (required):

Included in this Pull Request:

  • A note to the change log.

@ericangeles ericangeles requested a review from a team as a code owner June 18, 2020 10:44
Copy link
Member

@tmcconechy tmcconechy left a comment

Choose a reason for hiding this comment

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

This looks good but the design has a few more variations. I would suggest in priority:

  1. The example on https://www.figma.com/file/UTwbDbeB1Xv34Uz6js36L4/ids-widget-counts?node-id=0%3A1 "Icon on Count"
  2. "Informational button for truncation "
  3. "BI Colors" ? Can it be other colors in the 07 color range?
  4. Its ok if soho theme is also the line only and not solid color
  5. what if there is 2 or 6 rings?

Can skip:

  • "text based" examples

@tmcconechy
Copy link
Member

Calling this WIP as it needs a few more things.

@ericangeles
Copy link
Contributor Author

@tmcconechy you can take a look now. The CI will probably failed because of baseline image. But will fix it after.

@tmcconechy
Copy link
Member

Looking great. Maybe just two things now

  1. On the 4th example: User example let's change the alert to another icon (like success)
  2. On the 3rd example: Can we show icons on all 4 (error, orange alert, yellow alert , info?)
  3. Can we add two more with six , two, three ect and see if they work ok?

@tmcconechy
Copy link
Member

Looks like you got it all covered now 😄 Maybe just fix the test and we call it done.
If you want to perhaps some of these can go on seperate test pages. And fix the test that way. Up to you either way.

@ericangeles
Copy link
Contributor Author

@tmcconechy this should be good to go now. ☕

Copy link
Member

@tmcconechy tmcconechy left a comment

Choose a reason for hiding this comment

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

🍌

@tmcconechy tmcconechy merged commit 5641be4 into master Jun 24, 2020
@tmcconechy tmcconechy deleted the 3666-widget-count-uplift branch June 24, 2020 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Widget Count: Needs uplift fixes
3 participants