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] provide valid value for letter-spacing CSS property in template #2437

Merged
merged 2 commits into from
Sep 16, 2021

Conversation

Grawl
Copy link
Contributor

@Grawl Grawl commented Sep 16, 2021

No description provided.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@changeset-bot
Copy link

changeset-bot bot commented Sep 16, 2021

🦋 Changeset detected

Latest commit: 6988465

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
create-svelte Patch
TODO Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@bluwy
Copy link
Member

bluwy commented Sep 16, 2021

Context: % isn't valid for letter-spacing. Perhaps we could remove that line altogether so the style would be the same as before? A before and after screenshots would help too.

@Grawl
Copy link
Contributor Author

Grawl commented Sep 16, 2021

why not

@bluwy
Copy link
Member

bluwy commented Sep 16, 2021

Your fix might work too, but it's hard to tell how it would look like unless we spin up the template locally, so a screenshot could help speed up the review process.

@Grawl
Copy link
Contributor Author

Grawl commented Sep 16, 2021

I set 0.1em because I guess it was planned with some space between letters in header nav.

With letter-spacing: 0.1em:

localhost_27565_

With letter-spacing: 1%:

localhost_27565_ (1)

(not working letter-spacing rule)

@bluwy
Copy link
Member

bluwy commented Sep 16, 2021

Thanks for the screenshots! I think the new letter-spacing indeed looks nicer, 👍 from me

EDIT: I just realized the CI actually creates a preview of the demo app changes... Really sorry!

@benmccann benmccann changed the title fix letter-spacing in header [fix] provide valid value for letter-spacing CSS property in template Sep 16, 2021
@benmccann
Copy link
Member

Thanks @Grawl for the change and @bluwy for the review!

@benmccann benmccann merged commit 7c1c9bd into sveltejs:master Sep 16, 2021
sidharthv96 added a commit to sidharthv96/kit that referenced this pull request Sep 19, 2021
* 'master' of https://github.com/sveltejs/kit: (322 commits)
  Version Packages (next) (sveltejs#2438)
  [fix] revert sveltejs#2354 and fix double character decoding a different way (sveltejs#2435)
  chore: update dependencies (sveltejs#2447)
  [docs] add link to envPrefix option in env var FAQ (sveltejs#2445)
  Fix invalid changeset. Thanks GitHub bot :-p
  [feat] use the Vite server options in dev mode (sveltejs#2232)
  [fix] provide valid value for `letter-spacing` CSS property in template (sveltejs#2437)
  [docs] fix typo (sveltejs#2443)
  [fix] add svelte field when packaging (sveltejs#2431)
  Version Packages (next) (sveltejs#2428)
  [chore] update lockfile
  [fix] update to TS 4.4.3 (sveltejs#2432)
  [chore] add links to repository and homepage to package.json (sveltejs#2425)
  docs: use fragment for prefetching link (sveltejs#2429)
  [fix] encodeURI during prerender (sveltejs#2427)
  Version Packages (next) (sveltejs#2420)
  revert change to versioning during release workflow
  chore: update vite-plugin-svelte (sveltejs#2423)
  [chore] expose Body generic to hook functions (sveltejs#2413)
  [feat] adapter-node entryPoint option (sveltejs#2414)
  ...
@Grawl Grawl deleted the patch-1 branch September 21, 2021 15:04
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.

None yet

3 participants