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

Empty style tag added to head on menu open/close #78

Closed
thomasbjespersen opened this issue Aug 12, 2022 · 8 comments
Closed

Empty style tag added to head on menu open/close #78

thomasbjespersen opened this issue Aug 12, 2022 · 8 comments
Assignees
Labels
bug Something isn't working wontfix This will not be worked on

Comments

@thomasbjespersen
Copy link
Contributor

When opening or closing a menu, an empty style tag is added to the head.

STR:

  1. Open Dev Tools and select Elements (or click on page-> Inspect)
  2. Go to the Menu docs and open/close a menu
  3. Inspect the head in dev tools

Screen Shot 2022-08-11 at 7 39 13 PM

@thomasbjespersen thomasbjespersen added bug Something isn't working good first issue Good for newcomers labels Aug 12, 2022
@thomasbjespersen
Copy link
Contributor Author

Please hold off on this one until #41 is completed and merged.

@endigo9740 endigo9740 added the help wanted Extra attention is needed label Aug 14, 2022
@ghost
Copy link

ghost commented Aug 17, 2022

I have seen this same behavior for things like the lightswitch and accordion, but Not sure how many components overlap under the hood

@endigo9740
Copy link
Contributor

Good to know it's happening with the accordion too. VERY strange bug, but two things of note:

  • The next update contains a total rebuild of the accordion component. It now utilizes detail/summary tags. So it may no longer cause the issue since it's less JS heavy.
  • I'm not saying the LightSwitch is without issue. Frankly the code is bad (I know, I wrote it heh). But it may be worth checking if the issue persists in the new SvelteKit versions. Could have been a Svelte/SvelteKit bug. But either way a refactor and improvement to that component is overdue.

FYI I'll be implementing a PR for updating to the next.405+ version of SvelteKit today, so I'll try remember to check this as I test!

@ghost
Copy link

ghost commented Aug 17, 2022

Roger, I will note my project is Current with SvelteKit.

@endigo9740
Copy link
Contributor

endigo9740 commented Aug 17, 2022

FYI I've confirmed the following:

  • I can replicate the head/style element updates for the live/production version of Skeleton (v0.30.7)
  • I cannot reproduce the issues in the most current dev branch, which means the accordion issues are resolved.

Given this, I'll turn my attention to the Menu and LightSwitch components.

@endigo9740 endigo9740 removed good first issue Good for newcomers help wanted Extra attention is needed labels Aug 17, 2022
@endigo9740 endigo9740 self-assigned this Aug 17, 2022
@endigo9740
Copy link
Contributor

I was under the impression the LightSwitch was at fault here, so I've opted to refactor and redesign this a bit. It now looks and operates a lot like the versions on these sites:

Screen Shot 2022-08-17 at 6 08 18 PM

Screen Shot 2022-08-17 at 6 08 25 PM

Unfortunately this has not solved the problem with the HTML head style blocks being duplicated, however this now only occurs when interacting with Menu components. I'll turn my attention to to the Menu components tomorrow.

@endigo9740 endigo9740 linked a pull request Aug 17, 2022 that will close this issue
@endigo9740 endigo9740 reopened this Aug 17, 2022
@endigo9740
Copy link
Contributor

Keeping this open to address the Menu updates mentioned in the comment above.

@endigo9740 endigo9740 removed a link to a pull request Aug 17, 2022
@endigo9740
Copy link
Contributor

Finally came across this and identified it's the transition directive that's causing this to occur. We use transition on several components, including Menus, Alerts, etc.

sveltejs/svelte#4801

Gonna mark this CLOSED as I don't believe it's an issue we can fix on our end. I'd suggest we report it to Svelte if we can identify any negative impact from this.

@endigo9740 endigo9740 added the wontfix This will not be worked on label Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants