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.

Sign up for GitHub

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

Jump to bottom

[#2184] Fix Inconsistent Line Number Colours #2185

Merged
merged 20 commits into from
Apr 27, 2024
Merged

[#2184] Fix Inconsistent Line Number Colours #2185

merged 20 commits into from
Apr 27, 2024

Conversation

sopa301
Copy link
Contributor

@sopa301 sopa301 commented Apr 2, 2024 •
edited
Loading

Fixes #2184.

Proposed commit message

Fix Inconsistent Line Number Colours

A regression has occurred where the line numbers no longer have a
consistent grey color.

Let's fix the bug and add a test to catch further regressions of the
same nature.

Other information

@sopa301 sopa301 requested a review from a team April 2, 2024 12:28
jonasongg
jonasongg approved these changes Apr 5, 2024
View reviewed changes
Copy link
Contributor

@jonasongg jonasongg left a comment

Choose a reason for hiding this comment

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

LGTM!

@jonasongg jonasongg requested a review from a team April 5, 2024 17:28
@sopa301 sopa301 requested a review from MarcusTXK April 6, 2024 14:01
@sopa301 sopa301 requested a review from ckcherry23 April 13, 2024 11:13
asdfghjkxd
asdfghjkxd approved these changes Apr 18, 2024
View reviewed changes
Copy link
Contributor

@asdfghjkxd asdfghjkxd left a comment

Choose a reason for hiding this comment

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

LGTM!

@asdfghjkxd asdfghjkxd requested a review from a team April 18, 2024 11:30
ckcherry23
ckcherry23 approved these changes Apr 20, 2024
View reviewed changes
Copy link
Member

@ckcherry23 ckcherry23 left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this @sopa301!

Seems like this test case takes 2 seconds to run in the CLI which is reasonable. However, I had longer wait times (~12 seconds) when running using the automated browser for some reason. The test case also caused crashes of the Cypress app sometimes, possibly due to repeatedly getting the child elements. If this causes more issues in the future, perhaps we can consider limiting how many line numbers are tested instead of checking all of them.

This LGTM for now though!

@ckcherry23 ckcherry23 requested a review from a team April 20, 2024 09:10
vvidday
vvidday approved these changes Apr 23, 2024
View reviewed changes
Copy link
Contributor

@vvidday vvidday left a comment

Choose a reason for hiding this comment

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

LGTM

@asdfghjkxd asdfghjkxd merged commit 553f3f2 into reposense:master Apr 27, 2024
8 checks passed
@github-actions GitHub Actions
Copy link
Contributor

The following links are for previewing this pull request:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vvidday vvidday vvidday approved these changes

@ckcherry23 ckcherry23 ckcherry23 approved these changes

@asdfghjkxd asdfghjkxd asdfghjkxd approved these changes

@jonasongg jonasongg jonasongg approved these changes

@MarcusTXK MarcusTXK Awaiting requested review from MarcusTXK

Assignees
No one assigned
Labels
s.ToMerge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Unintended color change in line numbers
6 participants
@sopa301 @vvidday @MarcusTXK @ckcherry23 @asdfghjkxd @jonasongg