Fix browser back button not working on PipelineRun detail page#4826
Fix browser back button not working on PipelineRun detail page#4826tekton-robot merged 1 commit intotektoncd:mainfrom
Conversation
|
|
68156e7 to
3895950
Compare
AlanGreene
left a comment
There was a problem hiding this comment.
Thanks for the PR @chrism417. The TaskRunDetails change looks good but we need to avoid breaking changes to the PipelineRun component.
3895950 to
cd49523
Compare
|
Hi @AlanGreene, I pushed some changes based on your comments above, let me know if the latest push is correct now. Thanks! |
AlanGreene
left a comment
There was a problem hiding this comment.
Hi Chris, thanks again for the PR. From my local testing it doesn't look like the changes in the PipelineRun component are needed, the change to add replace: true in TaskRunDetails appears to be sufficient to fix the back button behaviour.
Did you identify a particular scenario that required the changes you made in the PipelineRun component? If so, please share some more details about this including a reproducing example if possible so I can better understand the impact.
cd49523 to
9785a31
Compare
Use replace navigation for auto-selected query params (pipelineTask, view) so they don't push extra history entries. The view=logs auto-set in TaskRunDetails created an infinite back-button loop: pressing back removed the param, which triggered the effect to re-add it. Fixes #4783
9785a31 to
2c8c4f6
Compare
|
Hi Alan, |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: AlanGreene The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Just updated, let me know. Thank you for taking the time to review it! |
|
/lgtm |
Changes
This fixes #4783.
When a task is selected on the PipelineRun detail page,
TaskRunDetailsauto-sets theview=logsquery param via auseEffect. This was pushing a new history entry each time, creating an infinite back-button loop: pressing back removed the param, which triggered the effect to re-add it.Fix
{ replace: true }inTaskRunDetailswhen auto-settingview=logsso it replaces the current history entry instead of pushing a new onegetViewChangeHandlerto accept and forward thereplaceoption/kind bug
Submitter Checklist
As the author of this PR, please check off the items in this checklist:
functionality, content, code)
/kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tepRelease Notes