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: use colors from @dhis2/ui-core #306

Merged
merged 5 commits into from
Jul 29, 2019
Merged

chore: use colors from @dhis2/ui-core #306

merged 5 commits into from
Jul 29, 2019

Conversation

janhenrikoverland
Copy link
Collaborator

@janhenrikoverland janhenrikoverland commented Jun 26, 2019 •
edited by edoardo
Loading

Depends on dhis2/analytics#33!

Use shared colors.

jenniferarnesen
jenniferarnesen requested changes Jun 27, 2019
View reviewed changes
Copy link
Collaborator

@jenniferarnesen jenniferarnesen left a comment

Choose a reason for hiding this comment

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

Can you try to use the theme from ui-core instead?

@edoardo edoardo changed the title chore: use colors from @dhis2/analytics chore: use colors from @dhis2/ui-core Jul 2, 2019
@edoardo
fix: temporary fix for i18next namespace separator issue
9c9b0ae 
The : character is the default namespace separator and causes i18next to
choke if present in a string.
Nothing we can do in the app as the i18next init is defined in d2-i18n
and not accessible from the app.
@edoardo
Copy link
Member

edoardo commented Jul 2, 2019

@cooper-joe provided the following color codes mapping between the colors file and ui-core theme
colors:

const accentPrimaryDark = '#004BA0' -> primary800
const accentPrimary = '#1976D2' -> primary600
const accentPrimaryLight = '#63A4FF' -> primary300
const accentPrimaryLightest = '#EAF4FF' -> primary050

const accentSecondaryDark = '#004C40' -> secondary800
const accentSecondary = '#00796B' -> secondary600
const accentSecondaryLight = '#48A999' -> secondary300
const accentSecondaryLightest = '#B2DFDB' -> secondary200
const accentSecondaryTransparent = '#D9ECEB' -> secondary100

const black = '#000000' -> grey900
const greyDark = '#494949' -> grey700
const grey = '#9E9E9E' -> grey500
const greyLight = '#E0E0E0' -> grey300
const blueGrey = '#ECEFF1' -> grey200
const lightBlue = '#BBDEFB' -> blue200
const snow = '#F4F6F8' -> grey050
const white = '#FFFFFF' -> white

const negative = '#E53935' -> red500
const warning = '#F19C02' -> yellow500
const positive = '#3D9305' -> green500
const info = '#EAF4FF' -> grey300

// TODO remove these?
const paleGrey = '#FAFAFA' -> grey100
const lightGrey = '#F5F5F5' -> grey100
const charcoalGrey = '#CCCCCC' -> grey400
const blue = '#004BA0' -> blue800

@edoardo
Copy link
Member

edoardo commented Jul 2, 2019

ui-core seems to cause a problem in the app build.

Failed to minify the code from this file: 
 	/home/travis/build/dhis2/data-visualizer-app/node_modules/@dhis2/ui-core/build/es/Button/index.js:9

See: https://facebook.github.io/create-react-app/docs/troubleshooting#npm-run-build-fails-to-minify

edoardo and others added 2 commits July 3, 2019 09:12
@edoardo
Merge remote-tracking branch 'origin/master' into chore-use-shared-co…
907dd7c 
…lors
@varl @edoardo
chore: let webpack compile @dhis2 scoped packages to es5 to satisfy u…
18c7435 
…glifyjs (#308)

To allow DV to use the ui-core components (which are shipped as ES6 code),
the App is responsible to compile the ui-core components into the target
of the App.

The problem is that UglifyJS does not support ES6, so we need to compile
the ES6 code to ES5 code before shipping it to the html-webpack-plugin[1] minifier,
which uses html-minifier[2] which finally runs the uglifyjs library[3] which crashes.

> uglify-js only supports JavaScript (ECMAScript 5).

To solve this we need to allow webpack to run babel on @dhis2-scope packages in
node_modules.

[1] https://github.com/jantimon/html-webpack-plugin#minification
[2] https://github.com/kangax/html-minifier
[3] https://github.com/mishoo/UglifyJS2
@edoardo edoardo requested a review from cooper-joe July 3, 2019 09:46
@edoardo
Copy link
Member

edoardo commented Jul 3, 2019

Found one more place with hardcoded colors: App.css.

@edoardo edoardo added the WIP label Jul 3, 2019
@jenniferarnesen
Copy link
Collaborator

@edoardo Approved. When you squash merge, could you include Viktor's explanation of the webpack changes in the commit description?

@edoardo edoardo merged commit c20913d into master Jul 29, 2019
@edoardo edoardo deleted the chore-use-shared-colors branch July 29, 2019 13:30
@dhis2-bot
Copy link
Contributor

🎉 This PR is included in version 33.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

This was referenced Jun 8, 2020
Closed
Closed
Closed
Closed
Closed
Closed
This was referenced Jun 17, 2020
Closed
Closed
@dhis2-bot
Copy link
Contributor

🎉 This PR is included in version 33.1.13 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jenniferarnesen jenniferarnesen jenniferarnesen approved these changes

@cooper-joe cooper-joe cooper-joe approved these changes

Assignees
No one assigned
Labels
released on @33.x released WIP
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@janhenrikoverland @edoardo @jenniferarnesen @dhis2-bot @cooper-joe @varl