Feat/rework config page #990
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…mpat` - update routes - remove unused code - add `NotFound` page (wip)
- fix type issues - remove redundant variable - fix tests (most of them)
- fix bug in `useSearchQueryParametersState`
- downgrade packages to match grafana `grafanaDependency` version
- create feature toggle for POC - create tab content container - adapt current config page to the tabbed view
- use `generateRoutePath` - remove done TODOs - update tests for `ProbeCard` - update tests for `NewProbe` - update tests for `EditProbe` - remove debug content from `TestRouteInfo`
- update not found usage for `EditProbe`
…-mvp # Conflicts: # src/types.ts
…mpat` - update routes - remove unused code - add `NotFound` page (wip)
- fix type issues - remove redundant variable - fix tests (most of them)
- fix bug in `useSearchQueryParametersState`
- downgrade packages to match grafana `grafanaDependency` version
- use `generateRoutePath` - remove done TODOs - update tests for `ProbeCard` - update tests for `NewProbe` - update tests for `EditProbe` - remove debug content from `TestRouteInfo`
- update not found usage for `EditProbe`
- lock grafana dependencies on 11.3.0 - add 404 page instead of redirect to Home
…page # Conflicts: # package.json # src/components/CheckEditor/FormComponents/ChooseCheckType.tsx # src/components/CheckForm/CheckForm.tsx # src/components/CheckList/CheckList.test.tsx # src/components/ProbeEditor/ProbeEditor.tsx # src/components/Routing.tsx # src/page/EditProbe/EditProbe.tsx # src/page/NewProbe/NewProbe.test.tsx # src/routes/test/TestRouteInfo.tsx # src/test/dataTestIds.ts # src/types.ts # yarn.lock
- update `yarn.lock`
# Conflicts: # package.json # src/components/Routing.tsx # src/test/dataTestIds.ts # src/types.ts # yarn.lock
- remove `@ts-ignore` - remove extra padding - remove random "Hello" element - shorten imports - remove unused `ROUTES`
w1kman
commented
Nov 19, 2024
| @@ -40,6 +40,7 @@ export function useCanReadSM() { | |||
|
|
|||
| // we've rolled this back to respect org roles | |||
| // this will change when we do proper plugin RBAC in the near future | |||
| // Note: this is used by `PluginConfigPage`, which is not wrapped in any app context | |||
There was a problem hiding this comment.
Just a friendly reminder
ckbedwell
requested changes
Nov 21, 2024
Comment on lines
108
to
112
| clipboard: css` | ||
| max-height: 500px; | ||
| margin-top: 10px; | ||
| margin-bottom: 10px; | ||
| `, |
There was a problem hiding this comment.
We should add a linting rule to use the object syntax for emotion so it stays type safe.
There was a problem hiding this comment.
The template literals allow for CSS validation (with a Jetbrains IDE).
So if we have to decide on one, writing CSS as CSS should be the preferred way IMO.
It's also cleaner with pseudo classes
|
|
||
| export function UninitializedTab() { | ||
| // For some reason the 'call-to-action' variant causes infinity loop in the test (if the image is shown) | ||
| const hideImage = process.env.NODE_ENV === 'test'; |
There was a problem hiding this comment.
Madness 🤕 I think this is okay to leave even though adding test specific code to production components feels smelly.
There was a problem hiding this comment.
It would be const hideImage = true in production, but I agree with the smellyness either way.
- heading order
- change h1 to h2 on plugin config page - remove `console.log` - move `AppInitializer`
|
The EDIT: I just noticed the Screen.Recording.2024-11-22.at.17.38.56.mov |
VikaCep
reviewed
Nov 22, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it: In order to facilitate coming features, we need to move away from using a single config page. Also, this PR creates a dedicated
pluginConfigconfig page.Which issue(s) this PR fixes: "Rework the ConfigPage so that it is tabbed (to prepare for "Secrets" tab)"
Part of #987
Special notes for your reviewer:
Blocked byreact-router-dom-v5-compatPR.Tabs
The app config page has been split into three tabs (General, Access tokens & Terraform).
General
synthetic-monitoring-datasourceto the list of data sourcessynthetic-monitoring-appplugin page (not the same as the app)Access tokens
Terraform
&& \) to make it look a bit cleaner but remain as functionalcomponents/Preformatted.tsx)<Preformatted />A new component was added to allow for some simple highlighting of content.
It's also used by
<Clipboard />The
isCodeprop is used for accessibility clarification (will wrap code in a<code />tagPlugin config page
A notable change is that we're no longer using the same component for the plugin config page as the app config page. The plugin config page is very much made to be just a
pluginConfigpage. To keep it simple and somewhat detached, it shares as little as possible with the app components.