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

moved add comment page into modal on analysis page #1319

Merged
merged 1 commit into from
Jan 29, 2025

Conversation

jstucke
Copy link
Collaborator

@jstucke jstucke commented Dec 13, 2024

No description provided.

@jstucke jstucke added the gui label Dec 13, 2024
@jstucke jstucke requested a review from dorpvom December 13, 2024 09:25
@jstucke jstucke self-assigned this Dec 13, 2024
@codecov-commenter
Copy link

codecov-commenter commented Dec 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.80%. Comparing base (058e49f) to head (516cadb).
Report is 24 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1319      +/-   ##
==========================================
- Coverage   92.42%   91.80%   -0.63%     
==========================================
  Files         379      376       -3     
  Lines       23661    21044    -2617     
==========================================
- Hits        21869    19319    -2550     
+ Misses       1792     1725      -67     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@maringuu maringuu left a comment

Choose a reason for hiding this comment

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

I'm not convinced that a modal is appropriate here.
It still hides part of the information displayed on the page of the file you are commenting on.
Why not add a text input field where you can type to the comments section?

Aside:
The modal is kind of narrow and can only be resized vertically but not horizontally.

@jstucke
Copy link
Collaborator Author

jstucke commented Jan 16, 2025

I'm not convinced that a modal is appropriate here. It still hides part of the information displayed on the page of the file you are commenting on. Why not add a text input field where you can type to the comments section?

Aside: The modal is kind of narrow and can only be resized vertically but not horizontally.

It was @dorpvom's idea to use a modal. I agree that a modal might not be the best choice (because you might want to look at the analysis page while commenting). Let's hear @dorpvom's opinion on this.

@dorpvom
Copy link
Collaborator

dorpvom commented Jan 22, 2025

I have opined

@jstucke
Copy link
Collaborator Author

jstucke commented Jan 28, 2025

I changed it, moved it out of the modal and integrated it into the table where the "add comment" button is located.

@maringuu maringuu self-requested a review January 29, 2025 10:04
Copy link
Collaborator

@maringuu maringuu left a comment

Choose a reason for hiding this comment

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

Nice! Lgtm

@maringuu maringuu merged commit ff1167e into master Jan 29, 2025
10 checks passed
@jstucke jstucke deleted the comments2.0-modal branch January 31, 2025 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants