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

Gobierto Visualizations / Translate categories #3745

Merged
merged 7 commits into from
Mar 3, 2021
Merged

Conversation

jorgeatgu
Copy link
Contributor

Closes #N

✌️ What does this PR do?

  • Translate the categories titles.
  • Change the color of the treemap.

🔍 How should this be manually tested?

Staging

👀 Screenshots

After this PR

translate

Copy link
Member

@ferblape ferblape left a comment

Choose a reason for hiding this comment

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

Why aren't we using Rails I18n files here? You are re-inventing the wheel

@@ -0,0 +1,74 @@
export const categoryTitleEng = [
Copy link
Member

Choose a reason for hiding this comment

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

these are not exactly the titles in English, these are the translation keys. I'm saying it because maybe in the future there are real English translations and they object should have this name (just like categoryTitlesCat and categoryTitlesEsp).

contractsDataMap.map(d => {
const { category_title } = d
/*If the contract isn't categorized, we include it in other*/
if (!category_title) {
Copy link
Member

Choose a reason for hiding this comment

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

We don't need this in the end, since they will never come as null (and if they do, we should know to fix it)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. Done!

@jorgeatgu
Copy link
Contributor Author

Why aren't we using Rails I18n files here? You are re-inventing the wheel

Ouch. Fixed.

@@ -309,7 +309,7 @@ export default {
} else if (deepLevel === 3 && scaleColor && depth === 2) {
return this.colors(contracts[this.firstDepthForTreeMap])
} else {
return '#12365b'
return '#118e9c'
Copy link
Member

Choose a reason for hiding this comment

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

Should this be dynamic to avoid what just happened?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@@ -1,7 +1,7 @@
import { scaleOrdinal } from 'd3-scale';

export function createScaleColors(values, arrayDomain) {
let colorsGobiertoExtend = ["#12365b", "#118e9c", "#ff766c", "#f7b200", "#158a2c", "#94d2cf", "#3a78c3", "#15dec5", "#6a7f2f", "#55f17b"]
let colorsGobiertoExtend = ["#118e9c", "#12365b", "#ff766c", "#f7b200", "#158a2c", "#94d2cf", "#3a78c3", "#15dec5", "#6a7f2f", "#55f17b"]
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we be using here CSS variables of the brand colors? I guess this are already defined.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. I've added the Gobierto colors to the css-confs as CSS vars.

@jorgeatgu jorgeatgu requested a review from furilo March 3, 2021 12:44
Copy link
Member

@ferblape ferblape left a comment

Choose a reason for hiding this comment

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

Much better!

@jorgeatgu jorgeatgu merged commit 1a35330 into master Mar 3, 2021
@jorgeatgu jorgeatgu deleted the categories_titles branch March 3, 2021 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants