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 Tool Tip for Bread Crumbs in Comparison View #1717

Merged
merged 12 commits into from
May 3, 2024

Conversation

Kr0nox
Copy link
Member

@Kr0nox Kr0nox commented Apr 18, 2024

This PR adds tooltips in breadcrumps.
In addition it adds an export of the start and end index of the tokenlist for each match. This change is backwards compatible with 5.0.0 and earlier.
Tooltip in 5.1.0:
grafik

Tooltip in 5.0.0:
grafik


Moreover it includes a rework of the tooltip component. It now can grow outside of a container that has overflow set to hidden, auto or scroll

Fixes #1668

@Kr0nox Kr0nox added enhancement Issue/PR that involves features, improvements and other changes minor Minor issue/feature/contribution/change report-viewer PR / Issue deals (partly) with the report viewer and thus involves web-dev technologies labels Apr 18, 2024
@Kr0nox Kr0nox linked an issue Apr 18, 2024 that may be closed by this pull request
@Kr0nox Kr0nox added this to the 5.1.0 milestone Apr 18, 2024
@Kr0nox Kr0nox changed the title rework tooltip component Add Tool Tip for Bread Crumbs in Comparison View Apr 18, 2024
Copy link
Contributor

@TwoOfTwelve TwoOfTwelve left a comment

Choose a reason for hiding this comment

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

Looks good to me

return new Match(firstFileName, secondFileName, startLineFirst, startColumnFirst, endLineFirst, endColumnFirst, startLineSecond,
startColumnSecond, endLineSecond, endColumnSecond, match.length());
return new Match(firstFileName, secondFileName, startLineFirst, startColumnFirst, startTokenFirst, endLineFirst, endColumnFirst,
endTokenFirst, startLineSecond, startColumnSecond, startTokenSecond, endLineSecond, endColumnSecond, endTokenSecond, match.length());
Copy link
Contributor

Choose a reason for hiding this comment

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

@tsaglam Maybe we should think about splitting up the Match class in the future. For now I would keep it as is, so we don't break our API more than necessary.

Copy link
Member Author

Choose a reason for hiding this comment

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

Was thinking the same, already have made a card for that on the board

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I agree. We could just do I class for token subsequences, and a match just takes two subsequences.

@Kr0nox
Copy link
Member Author

Kr0nox commented Apr 25, 2024

There is still one bug that happens on smaller screens where the view becomes scrollable to the right. I will fix this before we merge this fixed

@Kr0nox
Copy link
Member Author

Kr0nox commented Apr 25, 2024

change token ranges to token indexes of matches changed

@Kr0nox
Copy link
Member Author

Kr0nox commented Apr 25, 2024

Another bug with the new tooltips is that when the match list is scrolled tooltips get displayed wrong fixed

Copy link

sonarcloud bot commented Apr 29, 2024

Copy link

sonarcloud bot commented Apr 29, 2024

@tsaglam tsaglam merged commit d091182 into develop May 3, 2024
45 checks passed
@tsaglam tsaglam deleted the report-viewer/breadcrump-tooltips branch May 3, 2024 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issue/PR that involves features, improvements and other changes minor Minor issue/feature/contribution/change report-viewer PR / Issue deals (partly) with the report viewer and thus involves web-dev technologies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Tool Tip for Bread Crumbs in Comparison View
3 participants