-
Notifications
You must be signed in to change notification settings - Fork 19
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
Chore: fix deprecation issues #1063
base: main
Are you sure you want to change the base?
Conversation
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.
I'm glad I didn't say anything when you did the "noise" PR. I guess you saw the same stuff and had the same feeling, regarding the PR legacy warnings :D
LGTM 😍
font-weight: 500; | ||
min-height: ${theme.spacing(3)}; | ||
line-height: 1.21em; // To make wrapped text look nice | ||
|
||
&::after { | ||
display: 'block' : 'none'; | ||
display: 'block'; |
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.
display: 'block'; | |
display: block; |
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.
On line 142: display: 'none'
=> display: none
;
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.
On line 152: background-image: gray;
=> background-color: gray;
@@ -172,5 +172,6 @@ const getStyles = (theme: GrafanaTheme2) => ({ | |||
}); | |||
|
|||
function isMac() { | |||
// eslint-disable-next-line deprecation/deprecation -- doesn't seem to be a stable alternative yet |
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.
One alternative is to listen to both keys.
"Hold ctrl/cmd to ..."
OR use kbar
(used int grafana/grafana
). Psst! kbar
uses navigator.platform
to detect Mac 😅
Either way, I'd modify the code so that navigator.platform
can be undefined without the app exploding. With the simplest way being a try-catch
.
Problem
A few places in the app are using deprecated methods and functions.
Solution
Update the deprecated methods and remove the unused ones.