-
Notifications
You must be signed in to change notification settings - Fork 841
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
[Theme] Base theme setup #8030
[Theme] Base theme setup #8030
Conversation
cd61810
to
d19f6c1
Compare
ab1ae64
to
1596ae7
Compare
b81a336
to
7486e9a
Compare
- externally created, compiled and imported themes don't pass the class check anymore we can check for the shape additionally
- theme contains only cloned values from current theme
- ensures current state EUI
-removes circle dependency on main eui
build: ensure sass files are added to packed sub packages
e64597a
to
ce4e8e1
Compare
62ad98d
to
2532104
Compare
@@ -253,6 +256,8 @@ | |||
}, | |||
"peerDependencies": { | |||
"@elastic/datemath": "^5.0.2", | |||
"@elastic/eui-theme-borealis": "0.0.1", | |||
"@elastic/eui-theme-common": "0.0.1", |
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.
ℹ️ The theme-common
package is added as peerDependency for now while it's not actually released yet. Once released we should add it as actual dependency.
@@ -16,8 +16,9 @@ | |||
], | |||
"scripts": { | |||
"start": "cross-env BABEL_MODULES=false webpack serve --config=src-docs/webpack.config.js", | |||
"build:workspaces": "yarn workspaces foreach -Rti --from @elastic/eui-theme-common run build && yarn workspaces foreach -Rti --from @elastic/eui --exclude @elastic/eui --exclude @elastic/eui-theme-common run build", |
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.
❓ I'm wondering if we want to move this also to the root of our monorepo to have a global script to run initially similar to yarn
, Wdyt?
"license": "SEE LICENSE IN LICENSE.txt", | ||
"scripts": { | ||
"build:clean": "rimraf lib/", | ||
"build": "yarn build:clean && yarn build:compile && yarn build:compile:cjs && yarn build:types", |
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.
ℹ️ Currently there are no watchers setup yet to listen to changes in these standalone packages. That means that - for now - we need to manually update when e.g. running Storybook or docs.
This should be added for devX but is likely going to happen as a follow-up task.
💔 Build Failed
Failed CI StepsHistory
|
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.
I reviewed the code again and ran all common scripts locally to ensure they're all working.
As we already discussed, the changes look great! I don't have any objections, especially considering this is going to be merged to a feature branch.
Summary
closes #124
Important
This PR is currently still in
Draft
mode, as it needs alignment on some package/dependency specific setups/architectural choices.📖 See additional information here.
QA
/eui
: runbuild:workspaces
to build workspace dependencies then check that our common development tools/scripts work as expectedyarn storybook
andyarn build-storybook
yarn start
andyarn build-docs
/website
: runbuild:workspaces
and then test that EUI+ docs works as expected viayarn start
andyarn build
Deployments & theme switcher testing
By default the theme switcher is only enabled for
development
environments.Storybook: https://eui.elastic.co/pr_8030/storybook/?path=/story/navigation-euibutton--playground
EUI docs: https://eui.elastic.co/pr_8030/
EUI+: https://eui.elastic.co/pr_8030/new-docs/