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

Feat: log viewer query param #1006

Merged
merged 2 commits into from
Feb 26, 2025
Merged

Feat: log viewer query param #1006

merged 2 commits into from
Feb 26, 2025

Conversation

murilx
Copy link
Contributor

@murilx murilx commented Feb 25, 2025

Add a boolean query parameter logOpen to both /build/$buildId and /test/$testId that saves the open state of the log viewer in those pages and open it automatically when the link is open with the parameter set to true

Closes #994

How to test

  • Check if the query parameter is correctly set when the log viewer is opened/closed
  • Check if the log viewer automatically opens when opening a link with the query parameter set to true
  • Check that any non-boolean value will just make the query parameter defaults to false
  • Check if the alternative routes are working correctly with the new query parameter

@murilx murilx self-assigned this Feb 25, 2025
@murilx murilx marked this pull request as draft February 25, 2025 15:40
@murilx murilx marked this pull request as ready for review February 25, 2025 16:20
Copy link
Collaborator

@Francisco2002 Francisco2002 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@MarceloRobert MarceloRobert left a comment

Choose a reason for hiding this comment

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

LGTM, working well

Comment on lines 76 to 77
(open: boolean) =>
navigate({ search: s => ({ ...s, logOpen: open }), state: s => s }),
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit

Suggested change
(open: boolean) =>
navigate({ search: s => ({ ...s, logOpen: open }), state: s => s }),
(isOpen: boolean) =>
navigate({ search: s => ({ ...s, logOpen: isOpen }), state: s => s }),

Comment on lines 343 to 344
(open: boolean) =>
navigate({ search: s => ({ ...s, logOpen: open }), state: s => s }),
Copy link
Collaborator

Choose a reason for hiding this comment

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

ditto isOpen

@murilx murilx force-pushed the feat/log-viewer-query-param branch from db9a567 to 05ff183 Compare February 26, 2025 12:40
@murilx murilx merged commit 5661c86 into main Feb 26, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

If the logviewer panel is open, add a mark to the URL parameters
3 participants