Skip to content

Conversation

@mick88
Copy link
Contributor

@mick88 mick88 commented Feb 11, 2023

Current logic incorrectly shows "⌘ + K" in Chrome on Windows (user agent 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/110.0.0.0 Safari/537.36'). It does not contain "Mac".

The logic was introduced in #1236 and appears to be swapped - showing Windows/Linux shortcut on mac and vice-versa.

Current logic incorrectly shows "⌘ + K" in Chrome on Windows (user agent 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/110.0.0.0 Safari/537.36'). It does not contain "Mac".

The logic was introduced in django#1236 and appears to be swapped - showing Windows/Linux shortcut on mac and vice-versa.
Copy link
Member

@CuriousLearner CuriousLearner left a comment

Choose a reason for hiding this comment

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

LGTM 🌮

@CuriousLearner
Copy link
Member

Thank you @mick88 for your contribution.

Nice catch in there!

I've just approved the workflow to run, let's wait for the tests to pass.

I will also tag @pauloxnet to review this.

@pauloxnet pauloxnet merged commit 677f4cd into django:main Feb 11, 2023
@CuriousLearner
Copy link
Member

Thanks @pauloxnet !

@pauloxnet
Copy link
Member

@mick88 thanks for your patch. I've just merged it and will see it in the Django website after the next deploy made by @felixxm .

@mick88 mick88 deleted the patch-1 branch February 11, 2023 19:18
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.

3 participants