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

fix: move cursor pointer style on panel header to the clickable part #864

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ryanatkn
Copy link

@ryanatkn ryanatkn commented Nov 13, 2024

The playground console header's toggle button has no cursor: pointer but its noninteractive wrapper div does, this moves the style to the button. The current behavior makes the non-clickable part appear clickable and the button not.

An alternative would be to just delete cursor: pointer; if that was the desired aesthetic but it would be less clear on desktop what's clickable. I think a small improvement, that also makes just deleting the line better, would be to move the .panel-header wrapper's padding to be a child concern, allowing the .panel-heading button to have height: 100% to fill its container.

I tried the padding change but the component is used in two places and slotted sibling styles are a bit tricky to get right, and I couldn't test the other place because Firefox/Chromium on Linux are erroring with SvelteKitError: Not found: /node_modules/.vite/deps/bindings_wasm_bg.wasm and in the browser CompileError: WebAssembly.instantiate(): expected magic word ..., I think related to this Vite issue - vitejs/vite#8427

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR references an issue where it is discussed ahead of time.
  • Prefix your PR title with feat:, fix:, chore:, or docs:.
  • This message body should clearly illustrate what problems it solves.

Copy link

vercel bot commented Nov 13, 2024

@ryanatkn is attempting to deploy a commit to the Svelte Team on Vercel.

A member of the Team first needs to authorize it.

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.

1 participant