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

Global styles were lost in SvelteKit transition #3159

Open
dae opened this issue Apr 22, 2024 · 5 comments
Open

Global styles were lost in SvelteKit transition #3159

dae opened this issue Apr 22, 2024 · 5 comments

Comments

@dae
Copy link
Member

dae commented Apr 22, 2024

Our old pages like deck-options.html import an associated file like deck-options-base.scss in index.ts. That file defines global CSS styling that gets included in the output bundle, and affects how the page displays. It contains simple things like tweaks to the body margins, and also a bunch of Bootstrap CSS.

This pattern doesn't work with SvelteKit, because Svelte pages are expected to only style local elements, and be explicit when they target global tags.

For things like body margins, we can solve this by copying the relevant styling from the -base.scss file into files like DeckOptionsPage.svelte, and wrapping the clauses in :global(...). Though we probably should put it somewhere central like +layout.svelte, instead of tweaking it for each page.

The bootstrap styles are going to be more of a pain, since adding :global(...) to all the styling is not practical. Global CSS can be included in ts/app.html, but I don't think we can reference a .scss file there.

It looks like Svelte style tags used to have a global modifier, but it seems that no longer works.

Related: #3156

@krmanik
Copy link
Contributor

krmanik commented Apr 22, 2024

The button to switch between note and io in editor is not highlighted, so I have done following changes. It seems the global scss included. May be we should do this for other global scss.

diff --git a/ts/editor/editor-toolbar/ImageOcclusionButton.svelte b/ts/editor/editor-toolbar/ImageOcclusionButton.svelte
index 8ff8860b7..769068d3c 100644
--- a/ts/editor/editor-toolbar/ImageOcclusionButton.svelte
+++ b/ts/editor/editor-toolbar/ImageOcclusionButton.svelte
@@ -62,9 +62,11 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
     </DynamicallySlottable>
 </ButtonGroup>
 
-<style>
-    :global(.active-io-btn) {
-        background: var(--button-primary-bg) !important;
-        color: white !important;
+<style lang="scss">
+    :global {
+        .active-io-btn {
+            background: var(--button-primary-bg) !important;
+            color: white !important;
+        }
     }
 </style>
diff --git a/ts/svelte.config.js b/ts/svelte.config.js
index 948fa290e..f00b5eb44 100644
--- a/ts/svelte.config.js
+++ b/ts/svelte.config.js
@@ -1,6 +1,7 @@
 import adapter from "@sveltejs/adapter-static";
 import { vitePreprocess } from "@sveltejs/vite-plugin-svelte";
 import { dirname, join } from "path";
+import preprocess from 'svelte-preprocess';
 import { fileURLToPath } from "url";
 
 // This prevents errors being shown when opening VSCode on the root of the
@@ -9,7 +10,7 @@ const tsFolder = dirname(fileURLToPath(import.meta.url));
 
 /** @type {import('@sveltejs/kit').Config} */
 const config = {
-    preprocess: vitePreprocess(),
+    preprocess: [vitePreprocess(), preprocess()],
 
     kit: {
         adapter: adapter(

@dae
Copy link
Member Author

dae commented Apr 23, 2024

Thanks Mani, I'll explore this further when I have a chance. It might fix <style global>, but I'll need to check it doesn't impact build performance much.

@dae
Copy link
Member Author

dae commented May 17, 2024

The slowdown is sub-10%, which is acceptable, so I'll add preprocess() in. I'm not sure what the other part of the patch is about though, as the 'toggle mask editor' button already appears to be highlighted in the current main branch, even without this change. Have I missed something?

@dae
Copy link
Member Author

dae commented Jun 3, 2024

If I have, let me know :-)

By the looks of it, Svelte 5 has introduced support for :global {}, so it might be worth checking if we still need that preprocessor after we update.

@krmanik
Copy link
Contributor

krmanik commented Jun 3, 2024

I have not checked it yet, today I will check it, then tell.

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

No branches or pull requests

2 participants