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

Rework Code displaying #1275

Merged
merged 12 commits into from
Sep 6, 2023
Merged

Rework Code displaying #1275

merged 12 commits into from
Sep 6, 2023

Conversation

Kr0nox
Copy link
Member

@Kr0nox Kr0nox commented Aug 31, 2023

This PR reworks the components involved in code displaying, to improve code quality and fix some bugs. It also improved the code in some other areas.

The following bugs have been fixed:

  • Syntax highlighting in the comparison view seems to be broken for JDoc comments
  • Fix the indent of code with different line numbers
  • Matches sometimes requiere two clicks to become visible in the comparison view

We no longer highlight every line individually (LineOfCode.vue) and rather highlight the entire code in CodePanle.vue. Here we now display the code and line numbers in a table to ensure that all lines start at the indetation.
Storing matches and the connection scrolling to the matching Match have been simplified alot, by not storing a component to scroll to, but rather having the components expose a function for scrolling to a certain file/line

@Kr0nox Kr0nox requested a review from sebinside August 31, 2023 18:02
Copy link
Member

@sebinside sebinside left a comment

Choose a reason for hiding this comment

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

Here are some nitty-gritty details regarding the code view:

  • Please have a look at the bug report from sonarcloud whether or not it is applicable
  • Clicking on an entry from the top token list at the top should probably scroll to the code snippet in both views
  • The currently selected view (left or right) gets scrolled using the scroll wheel even if the mouse is over the other side which is probably confusing to the user

Apart from that, this PR can be merged. I also noticed, that at least for me, the current version of the code view of the develop branch is broken.

@sonarcloud
Copy link

sonarcloud bot commented Sep 1, 2023

[JPlag Report Viewer] SonarCloud Quality Gate failed.    Quality Gate failed

Bug C 1 Bug
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@Kr0nox
Copy link
Member Author

Kr0nox commented Sep 1, 2023

Please have a look at the bug report from sonarcloud whether or not it is applicable

One of them I fixed, for the other I would have said its not applicable since we are using a table as its inteded purpose to structure data (in our case code) and we dont want a header in this place. So the remaining issue is not applicable in my opinion

Clicking on an entry from the top token list at the top should probably scroll to the code snippet in both views

Chromium based browsers apperantly have a problem with smooth scrolling two divs. Edge especially since it just stops the scrolling in place. Fixed by not having smooth scrolling.

The currently selected view (left or right) gets scrolled using the scroll wheel even if the mouse is over the other side which is probably confusing to the user

I could only reproduce this bug in Edge. All other browsers do not have this issue. Beacause of this and since it is a problem I could also reproduce on other websites having two scrollable containers (also exclusivly on edge), I would have deamed this an issue with Edge.

@sonarcloud
Copy link

sonarcloud bot commented Sep 1, 2023

[JPlag Plagiarism Detector] Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@Kr0nox
Copy link
Member Author

Kr0nox commented Sep 1, 2023

Apart from that, this PR can be merged. I also noticed, that at least for me, the current version of the code view of the develop branch is broken.

I now what the error is. I could do a fixing PR. On the other hand these changes in the PR would be overwritten by this one anyways.

@sebinside sebinside merged commit 74d5b8d into develop Sep 6, 2023
25 of 26 checks passed
@sebinside sebinside deleted the report-viewer/rework-codePanel branch September 6, 2023 07:29
@Kr0nox Kr0nox mentioned this pull request Oct 28, 2023
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.

2 participants