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

Canvas: Chore clean up more betterer warnings #88010

Merged
merged 10 commits into from
May 21, 2024

Conversation

nmarrs
Copy link
Contributor

@nmarrs nmarrs commented May 17, 2024

Clean up almost all of the remaining betterer issues

The last two files / betterer issues will require a bit more extensive refactor that I will follow-up with to get rid of canvas's barrel files

Related to #87477

@nmarrs nmarrs added type/chore area/panel/canvas Issues related to canvas panel no-backport Skip backport of PR no-changelog Skip including change in changelog/release notes labels May 17, 2024
@nmarrs nmarrs added this to the 11.1.x milestone May 17, 2024
@nmarrs nmarrs self-assigned this May 17, 2024
@nmarrs nmarrs requested review from grafanabot and a team as code owners May 17, 2024 04:01
@nmarrs nmarrs requested review from Develer and adela-almasan and removed request for a team May 17, 2024 04:01
@@ -11,7 +11,7 @@ export const PanZoomHelp = ({}: StandardEditorProps<string, unknown, unknown, un

return (
<>
<HorizontalGroup className={styles.hGroup}>
<Stack direction="row">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pics of this change... currently no way to pass a custom class down to Stack 🤔

Before
before

After
after

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's a bit less tidy but doesn't seem like a big deal.

@@ -3,276 +3,253 @@ import { css } from '@emotion/react';
import { GrafanaTheme2 } from '@grafana/data';

export function getGlobalStyles(theme: GrafanaTheme2) {
return css`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the change I'm most worried about haha, but I think it should be ok.. but would appreciate a review of this (cc @adela-almasan 😬)

Copy link
Collaborator

@codeincarnate codeincarnate left a comment

Choose a reason for hiding this comment

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

Looks good to me! Would be nice to have someone more familiar with canvas take a look as well, but looks fine overall 😄

@nmarrs nmarrs merged commit 3d980f1 into main May 21, 2024
28 checks passed
@nmarrs nmarrs deleted the canvas-finish-betterer-clean-up branch May 21, 2024 03:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/frontend area/panel/canvas Issues related to canvas panel no-backport Skip backport of PR no-changelog Skip including change in changelog/release notes type/chore
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants