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

chore(colors): remove duplicate colors #7161

Merged
merged 2 commits into from
Aug 3, 2023
Merged

chore(colors): remove duplicate colors #7161

merged 2 commits into from
Aug 3, 2023

Conversation

jniles
Copy link
Collaborator

@jniles jniles commented Jul 23, 2023

This commit removes duplicate colors from the ColorService.

Closes #7018

jmcameron
jmcameron requested changes Jul 24, 2023
View reviewed changes
Copy link
Collaborator

@jmcameron jmcameron left a comment

Choose a reason for hiding this comment

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

I noticed this problem while converting the E2E tests. Thanks for working on it!

The changes mostly look good.

I tested it using the bhima_test DB. I noticed that the colors that are from i18n COLORS.* are capitalized. Could you please modify the .json file and uncapitalize them. Also I think GRAY is a duplicate that could be deleted.

Thanks!

client/src/js/services/color.js
@@ -6,17 +6,6 @@ ColorService.$inject = [
function ColorService($translate) {
const service = this;
service.list = [
{ name : $translate.instant('COLORS.AQUA'), value : '#00ffff' },
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you!

jmcameron
jmcameron requested changes Jul 25, 2023
View reviewed changes
Copy link
Collaborator

@jmcameron jmcameron left a comment

Choose a reason for hiding this comment

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

Thanks for the changes. I hate to be picky, but could I ask you to delete the duplicate 'gray' color and sort the list of colors alphabetically. And why do we have translations for some colors and not others (legacy?).

jniles added 2 commits August 3, 2023 09:42
@jniles
chore(colors): remove duplicate colors
5d3112f 
This commit removes duplicate colors from the ColorService.

Closes Third-Culture-Software#7018

chore(colors): adjust casing of colors

Removes the capitalization of letters in the color service.
@jniles
chore(colors): add additional colors, rm dupes
85cb675 
This commit removes duplicates in the color list, ensures it is sorted,
and adds the full additional colors to the color service.
@jniles jniles force-pushed the fix-color-service branch from dce9b7b to 85cb675 Compare August 3, 2023 14:42
@jniles
Copy link
Collaborator Author

jniles commented Aug 3, 2023

@jmcameron this is ready to go 👍

@jmcameron
Copy link
Collaborator

bors r+

bors bot added a commit that referenced this pull request Aug 3, 2023
@bors @jniles
Merge #7161
8630820 
7161: chore(colors): remove duplicate colors r=jmcameron a=jniles

This commit removes duplicate colors from the ColorService.

Closes #7018

Co-authored-by: Jonathan Niles <jonathanwniles@gmail.com>
@bors
Copy link
Contributor

bors bot commented Aug 3, 2023

Build failed:

@jniles
Copy link
Collaborator Author

jniles commented Aug 3, 2023

Looks like MySQL failed to initialize - I'll restart it.

bors r+

@bors
Copy link
Contributor

bors bot commented Aug 3, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit 201aea2 into Third-Culture-Software:master Aug 3, 2023
@jniles jniles deleted the fix-color-service branch August 3, 2023 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jmcameron jmcameron jmcameron requested changes

Assignees

@jmcameron jmcameron

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Duplicate colors in the color selector
2 participants
@jniles @jmcameron