-
Notifications
You must be signed in to change notification settings - Fork 36
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
Add color to language model #2715
Conversation
I am not sure whether I am doing it right or not. I would be glad to get some guidance :) |
Code Climate has analyzed commit 84d0e6e and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 100.0% (50% is the threshold). This pull request will bring the total coverage in the repository to 82.6% (0.0% change). View more on Code Climate. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much for the PR 💪
You are already in a good direction 😸 There are some suggestions (find in comments) 👍
@lunars97 |
0c2ad59
to
c9d70a5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 😸 Now we can adapt the matomo_api_client.py 💪 Find the comments please.
A new specification came after the Service-UI/UX-CMS monthly meeting on the last monday: only colors should be shown as choice which are not yet assigned to any language so one doesn't have to check the list every time when assigning a color to another language 🙏 |
Can I then remove blocked and ui/ux labels and continue working on it? |
@osmers If the language has no color, the blank choice will be shown and when clicked on the drop down, the choice appears including colors that are assigned to other languages but with note which language is using the color If the language has a color, it looks like this first After clicking on the drop dow In this way it's easier to swap colors (because one doesn't have to think about which color is free for a temporary assigment during the swap) and also you can see directly in the choice which color and language are paired.
@lunars97 I |
Looks good and I like the fact, that we can see which color belongs to which language. I would also suggest to make the color field mandatory so that we do not end up with a language without color, ok? |
@MizukiTemma thank you for improving it :) |
I am receiving test error. Could someone help me to identify the testing file which is causing current error. I would appreciate it a lot :) |
Yup, we can do that 👍 |
Sounds reasonable 👍 I've totally forgotten the issue 😅 |
I have added colors for WebApp Access and Offline Access to the original 🎨 figma file. Thanks for being so patient! |
@MizukiTemma hmm do we also need for Total Access? Currently we do not have a designated color for it, it uses default color, can I designate the default to total access then? EDIT: I assigned the default color for TotalAccess. If it is okay, then we can leave it as it is... |
You can also use black (#000000) for Total Access. I have added it to the design. |
6a08f3d
to
8a86bc1
Compare
63ebe99
to
84b3169
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you a lot for this PR. You did a very good job. I have a few suggestion and after that I think this PR is ready for release 🚀
9409657
to
9924d25
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you a lot! Looks good to me now :)
I didn't double-check all of the color names, only a few that didn't seem right to me. I hope that's okay and we only need to adjust it if we get some feedback :)
@lunars97 Thank you so much 😸 it looks good to go! Could you squash the commits and rebase? |
ce5c847
to
9924d25
Compare
9924d25
to
84d0e6e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
Short description
Add static color labels to language model in order to improve understanding of statistics regardless of to which region it belongs.
Proposed changes
Side effects
None?
Additional info
Should be done before #1097
Resolved issues
Fixes: #2390
Pull Request Review Guidelines