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

Add log line anchor for action logs #25532

Merged
merged 46 commits into from
Jul 3, 2023
Merged

Conversation

HesterG
Copy link
Contributor

@HesterG HesterG commented Jun 27, 2023

Close #24593

Some behavior:

  • If log step line in hash exists, expand the step and scroll to the log line.
  • If step exists but line not exists, the step will be expanded.
  • If step not exists, stays on the job's page.

Some Notes:

  • Changed mounted to async because need to await for first loadJob so currentJobStepsStates can be initialized and used in hashChangeListener .

Demo:

Untitled.mov

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jun 27, 2023
@HesterG HesterG added the topic/ui Change the appearance of the Gitea UI label Jun 27, 2023
@HesterG
Copy link
Contributor Author

HesterG commented Jun 29, 2023

I found scroll-margin-top working pretty well, and no need to use scrollIntoView after setting this. I set scroll-margin-top to 95px, which is about 5px below the fixed headers.

And changed href to not use absolute path. Updated demo in the description. Ready for review again.

@HesterG HesterG marked this pull request as ready for review June 29, 2023 05:52
@silverwind
Copy link
Member

And changed href to not use absolute path. Updated demo in the description. Ready for review again.

Should that fix #25532 (comment)?

@HesterG
Copy link
Contributor Author

HesterG commented Jun 29, 2023

#25532 (comment)

Yes

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jun 29, 2023
Copy link
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

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

Overall LGTM except the strange await loadJob, but it doesn't block

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jul 1, 2023
@lunny lunny added this to the 1.21.0 milestone Jul 1, 2023
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jul 1, 2023
@techknowlogick techknowlogick enabled auto-merge (squash) July 3, 2023 00:55
@techknowlogick techknowlogick merged commit 640a88f into go-gitea:main Jul 3, 2023
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jul 3, 2023
@HesterG HesterG deleted the log-hash branch July 3, 2023 01:09
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jul 3, 2023
* giteaofficial/main:
  use css on labels (go-gitea#25626)
  Get latest commit statuses from database instead of git data on dashboard for repositories (go-gitea#25605)
  Add log line anchor for action logs (go-gitea#25532)
  Support displaying diff stats in PR tab bar (go-gitea#25387)
  [skip ci] Updated licenses and gitignores
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Oct 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. topic/ui Change the appearance of the Gitea UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement actions log line anchor so that it's easy to jump to a special log line
6 participants