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

enhance(macros/AvailableInWorkers): add "only available in Web Workers" case #10968

Merged
merged 4 commits into from
May 14, 2024

Conversation

skyclouds2001
Copy link
Contributor

@skyclouds2001 skyclouds2001 commented Apr 22, 2024

Summary

the {{AvailableInWorkers}} macro that adds a message indicating a feature is available in web workers

in #10029, more cases is supported for the {{AvailableInWorkers}} macro

but there is still some cases that the {{AvailableInWorkers}} macro does not support

like:

  • only for Worker (like WorkerLocation WorkerNavigator and so on)

Problem

when working on mdn/content#31675, noticed that there's some cases that {{AvailableInWorkers}} macros can not cover

Solution

the macro's parameter support more available value
seom possible usage like: {{AvailableInWorkers("worker")}}


Screenshots

Before

None

After

image


How did you test this change?

Test by hand.

@github-actions github-actions bot added the macros tracking issues related to kumascript macros label Apr 22, 2024
Copy link
Member

@yin1999 yin1999 left a comment

Choose a reason for hiding this comment

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

LGTM for the zh-CN translation 👍

@skyclouds2001 skyclouds2001 marked this pull request as ready for review April 28, 2024 11:34
@skyclouds2001 skyclouds2001 requested a review from a team as a code owner April 28, 2024 11:34
@caugner
Copy link
Contributor

caugner commented May 3, 2024

@teoli2003 @bsmth @hamishwillee What do you think about this additional case?

@caugner caugner changed the title feat(macros/AvailableInWorkers): support one more distinct cases enhance(macros/AvailableInWorkers): add "only available in Web Workers" case May 3, 2024
@bsmth
Copy link
Member

bsmth commented May 3, 2024

@teoli2003 @bsmth @hamishwillee What do you think about this additional case?

LGTM, curious what others think. This should be every case covered by now, right?

@hamishwillee
Copy link
Contributor

This makes sense - I am sure there are at least a few worker-only features, in particular around things that have no UI.

This should be every case covered by now, right?

No, about half. The possible cases are all variations of: window, service worker, dedicated worker, shared worker - which is 2^4 = 16 variants.

But I suspect this covers the cases which make sense in the spec for now.

@skyclouds2001
Copy link
Contributor Author

But I suspect this covers the cases which make sense in the spec for now.

Yes, I think currently this has covered all cases in the spec for now

Copy link
Contributor

@caugner caugner left a comment

Choose a reason for hiding this comment

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

LGTM, just two nits that I will apply.

kumascript/macros/AvailableInWorkers.ejs Outdated Show resolved Hide resolved
kumascript/macros/AvailableInWorkers.ejs Outdated Show resolved Hide resolved
@caugner caugner enabled auto-merge (squash) May 14, 2024 13:33
@caugner caugner merged commit d56a76f into mdn:main May 14, 2024
10 checks passed
@skyclouds2001 skyclouds2001 deleted the macros branch May 14, 2024 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
macros tracking issues related to kumascript macros
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants