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

ui: Read source UI improvements #3670

Merged
merged 8 commits into from
Aug 24, 2023
Merged

ui: Read source UI improvements #3670

merged 8 commits into from
Aug 24, 2023

Conversation

manojVivek
Copy link
Contributor

@manojVivek manojVivek commented Aug 24, 2023

Fixes:

  • Eliminate redundant source requests for the same build id.
  • Clicking on a line number should highlight that line and persist it in the URL so that I can send the URL to someone, and it will scroll to that position
  • It seems that somehow the hover-bubble keeps re-rendering or something else that makes it feel laggy when having sources open
  • Horizontal scrollbar for the source viewer.
  • Fallbacks handled when the line number field is missing.

@manojVivek manojVivek requested a review from a team as a code owner August 24, 2023 09:20
@alwaysmeticulous
Copy link

alwaysmeticulous bot commented Aug 24, 2023

🤖 Meticulous spotted visual differences in 33 of 303 screens tested: view and approve differences detected.

Last updated for commit 394a58b. This comment will update as new commits are pushed.

Copy link
Contributor

@yomete yomete 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
Member

@brancz brancz left a comment

Choose a reason for hiding this comment

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

Amazing work!

@manojVivek manojVivek merged commit 9a4257a into main Aug 24, 2023
32 checks passed
@manojVivek manojVivek deleted the read-source-ui branch August 24, 2023 11:39
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.

3 participants