illright / attractions

A pretty cool UI kit for Svelte
https://illright.github.io/attractions
MIT License
1.03k stars 37 forks source link

:global() isn't processed in SvelteKit, some styles aren't applied #344

Closed yasserlens closed 2 years ago

yasserlens commented 2 years ago

Example: The Accordion component uses the :global() operator to apply styling to the up/down chevron to indicate the AccordionSection state (open or closed).

The styling for AccordionSection has logic that rotates the Chevron SVG to flip between up and down, which depends on the :global() operator.

According to this bug in Svelte (https://github.com/sveltejs/svelte/issues/2969), :global() isn't yet processed, which is why the :global() operator is useless / doesn't get applied successfully.

The suggestion is to rely less on :global() in styling. I understand the library is in maintenance mode (or not even that), just keeping this here for others so they wouldn't waste a couple of hours looking into this trivial issue :)

For AccordionSection specifically, I wish there was one event that fires on both open/close states which would have simplified handling the events to choose between icons (e.g. + and - instead of Chevron up/down). In Angular, I could have referenced the element with a # and accessed it's properties elsewhere within the html to get the "open" boolean property and display content conditionally based on that. There isn't an equivalent of this yet in Svelte (bind:this={something} doesn't work the same way).

aabounegm commented 2 years ago

The issue you mentioned is related only to customElements, and it says that preprocessing solves this issue. We currently do not support custom elements (I'm aware we haven't removed the respective page in the docs yet after we found the issues), and the installation page has a step to configure svelte-preprocess, which applies to SvelteKit as well (see the correct instruction here instead of the rollup one).

About the library being in maintenance mode, that is just because the development team is currently busy with our graduation projects, but we still actively check GitHub and welcome any contribution any time :)

As for referencing the element with an id and accessing its properties, the Svelte way to do this for the AccordionSection is using bind:open (see the component binding tutorial)

yasserlens commented 2 years ago

Thanks for the details.

Unless I'm missing something major - the current implementation doesn't seem to process :global() (using sveltejs/kit@1.0.0-next.326) and that is with preprocessors configured.

I tested this with an Accordion implementation, the symptom that made me investigate was that the Chevron did not rotate when the state of the AccordionSection changed from open<>close (which adds a class to AccordionSection, which is then used to rotate the chevron). I can see that the "open" class is added on state change, but the style isn't applied.

Thanks for the pointer to bind:open - I'll look into that now. I'd rather use +/- for the accordion so I'll need to use the open trigger to switch between two icons.

Good luck with your grad projects!

yasserlens commented 2 years ago

OK just realised that bind:open doesn't really solve the problem. I still need to bind to a unique variable per AccordionSection (or a property of an object) and I'm trying to avoid that. Thanks for the suggestion, I'll find another way.

aabounegm commented 2 years ago

I just tried creating a barebone SvelteKit project (via npm init svelte my-app), just added attractions (npm i -D attractions), copied the Accordion example from the docs, and it worked as it should, even with the chevron icon rotation. Can you please clarify more what problems are you having with styling, preferably sharing some code snippets?

As for bind:open, I just realized that it might be more appropriate to expose it as a slot prop on the AccordionSection component so you can let:open on it and use it in the template. Would that fix the problems you're having?

aabounegm commented 2 years ago

@yasserlens, is your problem solved? Can we close the issue?

yasserlens commented 2 years ago

@aabounegm I'm sorry for not commenting earlier. I decided not to use the library at the moment. It's one of the best I found out there and has great APIs, but I don't think its a good fit for my project at the moment. Thanks again for your timely responses and support.

aabounegm commented 2 years ago

That's understandable, and thanks for the kind words.