Skip to content

fix: add missing static css files for WC #1385

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

Merged
Lipata merged 11 commits into from
Apr 11, 2025
Merged

fix: add missing static css files for WC #1385

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
4fa2192
fix: add missing static css files
Apr 3, 2025
2879050
fix(wc): add all themes that exist on build
Apr 4, 2025
d07187d
fix(wc): fix name
Apr 4, 2025
9846fab
fix(wc): clear the added new logic in vite config
Apr 9, 2025
ac7d422
fix(wc): add typescript inline css handle file
Apr 9, 2025
e47ce76
chore(cli): release version 14.3.18-beta.0
Apr 10, 2025
98fa21e
resolve conflicts
Apr 10, 2025
d515d38
fix(wc): fix satrt script command and add new beta release verison
Apr 11, 2025
2c07420
chore(cli): release version 14.3.18-beta.2
Apr 11, 2025
60b519e
release: 14.3.8-beta.3
Lipata Apr 11, 2025
695d11b
chore: release version 14.3.18-beta.4
Apr 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions ...plates/webcomponents/igc-ts/projects/_base/files/__dot__github/workflows/github-pages.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,6 @@ jobs:
sed -i 's|<base href="[^"]*">|<base href="/${{ github.event.repository.name }}/">|' ./dist/index.html
- name: Update Resource Paths
run: find ./dist -maxdepth 1 -type f ! -name 'sw*.js' ! -name 'workbox*.js' -name '*.js' -exec sed -i -e "s|/src/assets|/${{ github.event.repository.name }}/src/assets|g" -e "s|url('/src/assets|url('/${{ github.event.repository.name }}/src/assets|g" {} +
- name: Copy ig-theme.css to dist
run: cp ./ig-theme.css ./dist/
- name: Update href Paths for ig-theme.css
run: |
find ./dist -type f -exec sed -i "s|href='../../ig-theme.css'|href='../../${{ github.event.repository.name }}/ig-theme.css'|g" {} +;
Expand Down
24 changes: 24 additions & 0 deletions packages/cli/templates/webcomponents/igc-ts/projects/_base/files/vite.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,18 @@
import { defineConfig } from 'vite';
import { VitePWA } from 'vite-plugin-pwa';
import { viteStaticCopy } from 'vite-plugin-static-copy';
import fs from 'fs';
import path from 'path';

const themeColors = ["light", "dark"];
const themeTypes = ["material", "bootstrap", "indigo", "fluent"];
const basePaths = [
"node_modules/igniteui-webcomponents-grids/grids/themes",
"node_modules/@infragistics/igniteui-webcomponents-grids/grids/themes"
];
const themeFiles = themeColors.flatMap(color =>
themeTypes.flatMap(theme => basePaths.map(basePath => `${basePath}/${color}/${theme}.css`))
).filter(fs.existsSync);
Copy link
Member

@damyanpetev damyanpetev Apr 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, nope :)

If the idea is to build out functionality to copy all themes in general, just copy them all - at best what you can do is hardcode the path, not the specific variants (material, etc, those can and will change).

Should be fine to just copy the entire node_modules/igniteui-webcomponents-grids theme folder and it'd do the same thing as this, right? Surely the static copy plugin can work with globs
Also, not sure why restricted to igniteui-webcomponents-grids as well, should include igniteui-webcomponents's themes too, no?

Copy link
Member

@damyanpetev damyanpetev Apr 4, 2025

Choose a reason for hiding this comment

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

Also we generally keep the package licensed maps separately so this could also work with one version of the packages and swap the node_modules path with the upgrade command. We don't put path maps in tsconfig for dual package support, so that will make it somewhat consistent.

Copy link
Contributor Author

@Hristo313 Hristo313 Apr 10, 2025

Choose a reason for hiding this comment

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

These changes are no longer needed, we will include these css files by importing them not by link tag.


export default defineConfig({
build: {
Expand All @@ -21,10 +33,22 @@ export default defineConfig({
chunkSizeWarningLimit: 10 * 1024 * 1024 // 10 MB
},
plugins: [
{
name: 'replace-grid-css-paths',
apply: 'build',
transform(code, id) {
if (id.endsWith('.js')) {
themeFiles.forEach(t => { code = code.replaceAll(t, `../../${path.basename(t)}`); });
return code;
}
}
},
/** Copy static assets */
viteStaticCopy({
targets: [
{ src: 'src/assets', dest: 'src' },
{ src: 'ig-theme.css', dest: '' },
Copy link
Member

@damyanpetev damyanpetev Apr 4, 2025

Choose a reason for hiding this comment

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

I understand why css from packages needs a static copy, but why the ig-theme which is part of the source. Is that the way vite instructs css to be handled, because I think this can also just be imported tp get it included.

Copy link
Member

@damyanpetev damyanpetev Apr 4, 2025

Choose a reason for hiding this comment

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

Wait.. that's not even it the project files? :O

Is that part for the CodeGen? Not sure it even belongs here if so, though you can argue it's functionality to allow to set global styling for all views; hint: the current styles.css is useless for that and your img, video styles and whatever missing resets we apply in other projects that are missing won't work from that either.

To that end, ig-theme can and should be in the templates with a comment explaining it sets up styles for all the views. The name also shouldn't really be ig-theme (that file actually had a different purpose initially) and should be included in all views generated by the CLI ofc.
Then you can justify the file handling in the project ;D

Copy link
Contributor Author

@Hristo313 Hristo313 Apr 10, 2025

Choose a reason for hiding this comment

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

Yes, ig-theme will be renamed and included in the templates folder by CodeGen

...themeFiles.map(themePath => ({ src: themePath, dest: '' }))
],
silent: true,
}),
Expand Down
Loading