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

feat: ecosystem themes with new default styles #5701

Merged
merged 5 commits into from
Nov 20, 2024
Merged

Conversation

emuvente
Copy link
Collaborator

This pulls in the latest changes from kv-ui-elements, which updates the default theme with ecosystem colors. Pairs with kiva/cms-page-server#1667.

The change to the exports field in the kv-components package.json required a change to how those components are imported. I was able to get the unit tests passing by changing the imports to the exact file name, e.g. import KvButton from '@kiva/kv-components/dist/components/KvButton.vue';, but that broke the linting rule that says we shouldn't include the file extension. Eslint also had problems recognizing the imports as we still use a commonjs-based eslint config.

Switching to the "flat config" method of configuring eslint and using ecma-script modules for it would require a lot of work, and it would still mean needing to use the full file name to import components. Instead of doing that, I decided to make an alias for the components directory in kv-components so that the import is just import KvButton from '#kv-components/KvButton';. After defining the alias in the eslint config, the jest config, the vite config, and after updating all the import statements, this allowed the linting, unit testing, and build process to succeed.

In the future we could consider changing the build process for kv-components to use vite in library mode, which would generate a javascript bundle for the components. That would allow us to import components like this import { KvButton, KvLoadingPlaceholder } from '@kiva/kv-components'; but we wouldn't have access to the single-file components anymore, and that might affect server rendering performance.

@emuvente emuvente merged commit 7d289db into main Nov 20, 2024
5 checks passed
@emuvente emuvente deleted the ecosystem-themes branch November 20, 2024 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants