Addons-frontend: Category buttons do not use proper palette

[pwalm]

The category buttons on the top level Extensions and Themes pages do not use the full palette provided.

Current:

Mocks:

Palette:

[muffinresearch]

@pwalm currently marking this for fix-up post-release.

[jvillalobos]

@willdurand can you look into what is needed to address this issue? Is there a direct mapping from categories to colors, or are they assigned in some other way?

[willdurand]

@willdurand can you look into what is needed to address this issue? Is there a direct mapping from categories to colors, or are they assigned in some other way?

@jvillalobos We have a list of 12 colors that we can update with Photon colors and that would solve the issue. This seems uncomplicated, so probably a good first bug.

https://github.com/mozilla/addons-frontend/blob/13b1ed0dc7aeca9994181596cc563baf387aab1a/src/ui/css/vars.scss#L39-L41

[jvillalobos]

How are they assigned? Stuart pointed out how themes only get green colors, which is odd.

[rahulsyal2512]

is this issue still open ? i can work with this one

[willdurand]

How are they assigned? Stuart pointed out how themes only get green colors, which is odd.

We use a simple function for that: https://github.com/mozilla/addons-frontend/blob/e4668770d1329012480c853749fda32e424ab0ab/src/core/utils/index.js#L247

This likely does not work for some categories because it uses the category ID to determine the color.

[jvillalobos]

I suggest we do two things:

1) Expand $category-colors to include all colors in the palette provided above.
2) Use a modulo operation in the category ID to decide which color should be assigned. Categories tend to have sequential numbering, so that should provide some variability in the colors. I bet the theme category IDs are entering the if (category.id > maxColors) { condition and then the division puts them all pretty much in the same place.

Still a good first bug, in my opinion. Thoughts?

[willdurand]

Still a good first bug, in my opinion. Thoughts?

I agree.

[jvillalobos]

Mentor: @kumar303

@rahulsyal2512 yes, you can take it.

[sashaktpathak]

If anybody has not taken up this issue, can i work with this one?
Also, Where is this function used further.

https://github.com/mozilla/addons-frontend/blob/e4668770d1329012480c853749fda32e424ab0ab/src/core/utils/index.js#L247

[rahulsyal2512]

i have added all the colors given in the pallete above.
please tell if there is anything to do with modulo operation?

[kumar303]

@rahulsyal2512 great, thanks. Did you create a pull request? I can take a look once you create a pull request.

[rahulsyal2512]

@kumar303 yes i did

[kumar303]

@rahulsyal2512 great, can you send me a link to the pull request? I'll take a look.

[rahulsyal2512]

@kumar303 here it is - https://github.com/mozilla/addons-frontend/pull/8139

[kumar303]

@rahulsyal2512 your pull request description is missing some important info that was there when you created it (see the template here). Can you add that back? Most importantly, it needs Fixes #3594 at the top.

[rahulsyal2512]

@kumar303 please check if this is fine now? https://github.com/mozilla/addons-frontend/pull/8146

[kumar303]

@rahulsyal2512 you still need to edit the description of the PR so that it says Fixes #3594 (it's the second checkbox item). This is important because it will automatically close the issue when the PR gets merged.

[kumar303]

@rahulsyal2512 sorry, I realize that the PR template is a bit confusing here. See the last sentence of the template:

"Once you have met the above requirements please replace this section with a Fixes #ISSUENUM linking to the issue fixed by this PR along with an explanation of the changes. Thanks for your contribution!"

[rahulsyal2512]

@kumar303 done

[rahulsyal2512]

i hope that will work now

[rahulsyal2512]

@kumar303 please merge the PR if there is no issue: - https://github.com/mozilla/addons-frontend/pull/8146

[kumar303]

@rahulsyal2512 thanks for all your work on this issue but, unfortunately, the issue was pretty far out of date. The mocks and guidance in the description are no longer valid. I made a new issue for what we need to do but that one is not a good first bug as it's no longer such a straight forward fix.

Moved to https://github.com/mozilla/addons-frontend/issues/8180