Skip to content

Convert static styles/properties getter to class fields #495

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

Merged
merged 5 commits into from
Sep 13, 2021
Merged

Conversation

aomarks
Copy link
Member

@aomarks aomarks commented Sep 10, 2021

  • Convert static styles getters to static class fields
  • Convert static properties getters to static class fields
  • Replace invalid curly quotes with straight quotes in code samples
  • Update localization docs for JS

@github-actions
Copy link

github-actions bot commented Sep 10, 2021

A live preview of this PR will be available at the URL(s) below.
The latest URL will be appended to this comment on each push.
Each build takes ~5-10 minutes, and will 404 until finished.

https://pr495-b3b17ea---lit-dev-5ftespv5na-uc.a.run.app/
https://pr495-e557be6---lit-dev-5ftespv5na-uc.a.run.app/

];
}
static styles = [
super.styles || [],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
super.styles || [],
super.styles ?? [],

@arthurevans
Copy link
Contributor

Have we decided that this style is OK now? When we released the first RC, the call was still to use the static getters because static class fields were problematic... somewhere. Yeah, I wish I remembered, too. Maybe @justinfagnani does.

@aomarks
Copy link
Member Author

aomarks commented Sep 13, 2021

Have we decided that this style is OK now? When we released the first RC, the call was still to use the static getters because static class fields were problematic... somewhere. Yeah, I wish I remembered, too. Maybe @justinfagnani does.

Static class fields were added in Chrome 72 (January 2019), Firefox 75 (April 2020), Safari 14.1 (May 2021).

So I assume the decision was made because Safari didn't have them yet when the lit.dev docs were first written, but now it does.

@arthurevans
Copy link
Contributor

Yep--that was it. Thanks, Al!

@aomarks aomarks merged commit 3f21d9d into main Sep 13, 2021
@aomarks aomarks deleted the more-js-docs branch September 13, 2021 18:44
aomarks added a commit that referenced this pull request Sep 14, 2021
This actually broke in #495
where I forgot to update these ranges.
aomarks added a commit that referenced this pull request Sep 14, 2021
This actually broke in #495
where I forgot to update these ranges.
aomarks added a commit that referenced this pull request Sep 14, 2021
This actually broke in #495
where I forgot to update these ranges.
aomarks added a commit that referenced this pull request Sep 14, 2021
This actually broke in #495
where I forgot to update these ranges.
aomarks added a commit that referenced this pull request Sep 14, 2021
- Replace home-page-specific language toggle with the new global one.

- Replace the separate JavaScript vs TypeScript "Hello World" Playground examples with a unified one. I'm keeping the JS output hand-rolled for this example for now, because the `static properties` block isn't in the perfect position, and this is a very prominent example. Will fix in lit/lit#2159 as followup.

- Fix home page highlight positions when in JS mode. This actually broke in #495 where I forgot to update the ranges for the yellow-highlight effect.

- Added a check for generated samples that have lost or re-ordered playground fold/hide comments. This should help detect weird cases that might need hand-rolling going forward.

- Hand-roll two examples that are losing fold comments, and move one comment to be trailing so that it's not lost, that I found via the above check. Filed lit/lit#2158 to hopefully address in the transformers in the future.

- Remove some unnecessary TypeScript vs JavaScript language, and add a few more switchable samples.

- Remove `jsSamples` mod feature flag to launch JS docs.

Fixes #332
Fixes #490
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.

4 participants