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
DashboardScene: Fixing major row repeat issues #87539
Conversation
private handleRepeatOptionChanges(panelRepeater: DashboardGridItem) { | ||
let width = panelRepeater.state.width ?? 1; | ||
let height = panelRepeater.state.height; | ||
private commitChangesToSource(gridItem: DashboardGridItem) { |
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.
this is unrelated change, and is only here for readability. I have numerous times been very confused trying to find where panel changes are committed (thinking this function only handled repeat options)
Updated this branch so that it contains all the scene fixes via this temp PR grafana/scenes#732 , makes it easier to test and verify the fixes |
This PR must be merged before a backport PR will be created. |
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.
👋🏾 Hey @torkelo , I did a quick test with one of the gdev-dashboards
(http://localhost:3000/d/0OmtTCtnk/repeating-a-row-with-a-repeating-horizontal-pane) and found an error of repeating panels disappearing after trying to move them 🤔
ErrorRepeatingHorizontally.mp4
@axelavargas think that might need a fix in scenes lib, to block the movement of panel from the source row to a clone row |
@axelavargas think we can fix that later, looks tricky to fix (and not as bad as the current bugs where panels disappear when you move them into the first source row). Moving panel out of the first row and into repeated clone is not really a usecase, more a user mistake. We should fix it but could not find an easy way to block/cancel dropping panel into a row clone, maybe @mdvictor can take a look at it he did something similar a month a go |
Could not replicate this (with a row with a normal non repeated panel inside it). |
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.
Hello 😄 , after more testing I did not find anything else.
P.S The added unit test made the process of discovering/understanding repeats
code easier🏅
Co-authored-by: Dominik Prokop <dominik.prokop@grafana.com>
This PR must be merged before a backport PR will be created. |
… into scene-row-repeat-rethink
… into scene-row-repeat-rethink
The backport to
To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-87539-to-v11.0.x origin/v11.0.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x 9cd7c87b484d66448144ab12a1e3915199fc7f6a When the conflicts are resolved, stage and commit the changes:
If you have the GitHub CLI installed: # Push the branch to GitHub:
git push --set-upstream origin backport-87539-to-v11.0.x
# Create the PR body template
PR_BODY=$(gh pr view 87539 --json body --template 'Backport 9cd7c87b484d66448144ab12a1e3915199fc7f6a from #87539{{ "\n\n---\n\n" }}{{ index . "body" }}')
# Create the PR on GitHub
echo "${PR_BODY}" | gh pr create --title "[v11.0.x] DashboardScene: Fixing major row repeat issues" --body-file - --label "type/bug" --label "area/frontend" --label "add to changelog" --label "backport" --base v11.0.x --milestone 11.0.x --web Or, if you don't have the GitHub CLI installed (we recommend you install it!): # Push the branch to GitHub:
git push --set-upstream origin backport-87539-to-v11.0.x
# Create a pull request where the `base` branch is `v11.0.x` and the `compare`/`head` branch is `backport-87539-to-v11.0.x`.
# Remove the local backport branch
git switch main
git branch -D backport-87539-to-v11.0.x |
* DashboardScene: Fixing major row repeat issues * Fixing edit scope * Use dashboard variableDependendency to notify row repeat behaviors * update scenes lib * Do not repeat if values are the same * Update public/app/features/dashboard-scene/scene/DashboardScene.tsx Co-authored-by: Dominik Prokop <dominik.prokop@grafana.com> * Updated scenes * Update * Update * Do not render row actions for repeated rows * Fixed e2e --------- Co-authored-by: Dominik Prokop <dominik.prokop@grafana.com> (cherry picked from commit 9cd7c87)
DashboardScene: Fixing major row repeat issues (#87539) * DashboardScene: Fixing major row repeat issues * Fixing edit scope * Use dashboard variableDependendency to notify row repeat behaviors * update scenes lib * Do not repeat if values are the same * Update public/app/features/dashboard-scene/scene/DashboardScene.tsx Co-authored-by: Dominik Prokop <dominik.prokop@grafana.com> * Updated scenes * Update * Update * Do not render row actions for repeated rows * Fixed e2e --------- Co-authored-by: Dominik Prokop <dominik.prokop@grafana.com> (cherry picked from commit 9cd7c87)
Noticed several bugs with row repeats.
See issues here: https://ops.grafana-ops.net/d/ddl5wm680ka2of/repeating-row-t?orgId=1&scenes&var-server=A&var-server=B&var-server=C&tab=queries
Bugs noticed (all fixed)
Changes / Fixes
Unsolved bugs that I do not know how to fix right now, I think we can fix later
Both those two bugs are because the first row (The template source) all all referenced by their plain ids / keys. Not really sure how to fix them yet. But given the bugs this PR fixes that are much more critical I think we can wait to fix them separately.
Maybe in a future PR (this was what I originally started working on this morning)
Fixes: #87536