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

Update platform detection #639

Merged
merged 2 commits into from
Dec 21, 2024
Merged

Conversation

ibretsam
Copy link

@ibretsam ibretsam commented Dec 20, 2024

Navigator.platform is deprecated. We can use Navigator.userAgentData or Navigator.userAgent to get the platform data.

But since userAgentData is not supported on some browsers, I suggest we should use userAgent instead.

For example, the userAgent value for some browser on MacOS looks like this:

// Chrome
'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/131.0.0.0 Safari/537.36'

// Safari
"Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.5 Safari/605.1.15" = $1

// Firefox
"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:133.0) Gecko/20100101 Firefox/133.0" 

Limitations:
While userAgent isn't exactly used for OS detection like platform, it's acceptable in this scenario since most modern browsers include the operating system information in the userAgent string.

@@ -22,6 +22,58 @@ export class Key {
static readonly Z = 90;
}

export function getKeyFromEvent(e: KeyboardEvent): number {
Copy link
Owner

@tuanchauict tuanchauict Dec 21, 2024

Choose a reason for hiding this comment

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

This is not good for maintenance. For example, when we add new supported key, we have to list the key twice.

One option is changing getCommandByKey to accept the key value.

The other is combine key and keyCode into the same record

{
    key: 'Escape',
    code: 27
}

@tuanchauict
Copy link
Owner

Btw, this PR is for Update platform detection, not for updating the keyCode. Please drop 62f6d72

@tuanchauict tuanchauict merged commit 2063d4f into tuanchauict:port-to-js Dec 21, 2024
7 checks passed
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.

2 participants