-
Notifications
You must be signed in to change notification settings - Fork 5k
feat: update API/SFC preference using shortcuts #1449
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
feat: update API/SFC preference using shortcuts #1449
Conversation
|
✔️ Deploy Preview for vue-docs-preview ready! 🔨 Explore the source changes: 9113684 🔍 Inspect the deploy log: https://app.netlify.com/sites/vue-docs-preview/deploys/61f1291fc3d6000007ca34a6 😎 Browse the preview: https://deploy-preview-1449--vue-docs-preview.netlify.app |
|
@skirtles-code, this one! |
|
Can we get rid of the dotted underline? It looks a bit out of place. We can mention the shortcut at the end of the "Introduction - API Styles" section . |
Yes, I'll do that |
Postpone preference changes when it's closed to the change in action
|
I just tried on OSX, and shortcut doesn't work for an unknown reason.. |
Fixed! |
|
@edimitchel Great 👍 |
create usePlateform
✅ Deploy Preview for vuejs ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
|
Preview URL: https://deploy-preview-1449--vuejs.netlify.app/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here are a few small suggestions from me based on an initial read through.
I'd also note that I'm on Ubuntu, which uses Ctrl+Alt+T to launch a terminal window, so I can't actually use that shortcut to toggle the API. But I'm not sure whether that's worth addressing for the relatively small number of people affected.
skirtles-code
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GitHub seems to be having a lot of problems today. For some reason this review comment didn't get included with my review, so here it is separately:
| ? JSON.parse(localStorage.getItem(key) || String(defaultValue)) | ||
| : defaultValue | ||
|
|
||
| export const preferCompositionKey = 'vue-docs-prefer-composition' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could some of these uses of export const be switched to const? It looks like some of them aren't need outside this file.
Decided to remove the SFC Preference shortcut which don't making sense, unlink the API Preference shortcut |
| isOpen, | ||
| shortcutInfo, | ||
| showPreference, | ||
| showSFC, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that showSFC is no longer being returned, so the second switch is currently missing on the Tutorial and Examples pages.
| @@ -0,0 +1,9 @@ | |||
| export default function usePlatform() { | |||
| const isMac = /(Mac OS X)/i.test(globalThis.navigator?.userAgent) | |||
| const altKey = isMac ? 'Option' : 'Alt'; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| const altKey = isMac ? 'Option' : 'Alt'; | |
| const altKey = isMac ? 'Option' : 'Alt' |
|
@edimitchel I'd like to get this over the line. Could you take a look at my latest comments above? There are also merge conflicts with the current |
|
Closing this PR as it's not adding as much functionality as I thought |
Description of Problem
Easy the switch without using mouse and keep focus on content
Proposed Solution
Add keyup event listener for API and SFC preference.

Additional Information
Externalize logic into a composable