-
Notifications
You must be signed in to change notification settings - Fork 188
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
Display the correct introduced event tooltip #2341
Display the correct introduced event tooltip #2341
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I slightly updated the suggested text for the tooltip to get it fit a single line. I'm open to suggestions though.
<span class="tooltiptext">Unknown commit / All previous commits are affected</span> | ||
{% else -%} | ||
<span class="tooltiptext">Unknown version / All previous versions are affected</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's important to have the "introduced" in there, can we change the max-width in the css to make it fit on one line? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -219,24 +219,26 @@ <h3 class="mdc-layout-grid__cell--span-3"> | |||
<dd> | |||
<div class="mdc-layout-grid__inner events"> | |||
{% for event in range.events -%} | |||
<div class="mdc-layout-grid__cell--span-3"> | |||
<div class="mdc-layout-grid__cell--span-2"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are the layout width changes intentional? I do prefer to keep the width the same here if possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@another-rex Ah, yesterday, I had an afternoon brain and tried a few options, but none worked. Just re-tried and now it's been fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
5b9c8d4
to
3a33842
Compare
Thanks! I think just merging in the main branch should fix the PR check. |
Prior to this change, when the introduced version in the affected range was 0, the tooltip displayed: "The exact introduced commit is unknown". This change, updates the html block to be more readable, and displays a more descriptive tooltip based on the type of the range, indicating if it has versions or commits.
3a33842
to
d713932
Compare
Prior to this change, when the introduced version in the affected range was 0, the tooltip displayed: "The exact introduced commit is unknown".
This change, updates the html block to be more readable, and displays a more descriptive tooltip based on the type of the range, indicating if it has versions or commits.
resolves: #2336
Sample output for GIT range:
Sample output for other type of ranges: