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

Incorporate client suggestions for feedback page #222

Merged
merged 13 commits into from
Jan 16, 2025

Conversation

MaHaWo
Copy link
Collaborator

@MaHaWo MaHaWo commented Jan 14, 2025

  • Deviations from design in Update of the feedback page #223
    • keep the symbols instead of colored uniform dots or just colored elements because colorblind people would have a hard time otherwise
    • kept the dropdown on the legend to avoid a bit of scrolling on mobile.
    • hide explanation of symbols behind buttons and only display name to avoid space problems on smaller screens.
  • I went with a minimally formatted version of the printed report at the moment. We can adjust this as desired.

Done:

  • add coloring
  • use gridlayout for milestonegroups
  • redo legend
  • change text according to template
  • add report printing

@codecov-commenter
Copy link

codecov-commenter commented Jan 14, 2025

Codecov Report

Attention: Patch coverage is 0% with 124 lines in your changes missing coverage. Please review.

Project coverage is 29.76%. Comparing base (08777e5) to head (82377f7).

Files with missing lines Patch % Lines
...rontend/src/lib/components/ChildrenFeedback.svelte 0.00% 92 Missing ⚠️
frontend/src/lib/translations.ts 0.00% 24 Missing ⚠️
frontend/tailwind.config.ts 0.00% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #222      +/-   ##
==========================================
- Coverage   30.30%   29.76%   -0.55%     
==========================================
  Files         115      115              
  Lines        4421     4502      +81     
  Branches      100      100              
==========================================
  Hits         1340     1340              
- Misses       2997     3078      +81     
  Partials       84       84              

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

@MaHaWo MaHaWo requested review from Majapur1 and lkeegan January 14, 2025 15:44
@MaHaWo
Copy link
Collaborator Author

MaHaWo commented Jan 14, 2025

sorry. review request was early.

@MaHaWo MaHaWo self-assigned this Jan 15, 2025
@MaHaWo MaHaWo linked an issue Jan 15, 2025 that may be closed by this pull request
@MaHaWo
Copy link
Collaborator Author

MaHaWo commented Jan 15, 2025

@Majapur1 @lkeegan I think this is ready for review now. Could you have a look when you have time? @Majapur1 Can you please check if the deviations from your design are ok and if I overlooked something?

@Majapur1
Copy link
Collaborator

@Majapur1 @lkeegan I think this is ready for review now. Could you have a look when you have time? @Majapur1 Can you please check if the deviations from your design are ok and if I overlooked something?\

How can I have a look at it visually on my computer (or on Github)? @lkeegan

@MaHaWo
Copy link
Collaborator Author

MaHaWo commented Jan 15, 2025

have you had a look at the README.md files in the code? There is one on the toplevel one in the frontend folder and one in the backend folder. These should help you a little bit.

@lkeegan
Copy link
Member

lkeegan commented Jan 15, 2025

@Majapur1 @lkeegan I think this is ready for review now. Could you have a look when you have time? @Majapur1 Can you please check if the deviations from your design are ok and if I overlooked something?\

How can I have a look at it visually on my computer (or on Github)? @lkeegan

The best way imo would be for you to run the whole thing locally - this should be fairly straightforward to do using docker:

https://github.com/ssciwr/mondey/blob/main/DEVELOPMENT.md#run-locally-with-docker

Then you can sign-up as a user on your local version of the website, and whenever you want to see what a branch looks like you can checkout the branch and re-build the docker containers.

If this isn't an option, then an alternative would be for us to merge these changes, update the running deployed website, then you can try it out there and make a new issue if you have any suggestions or changes.

(A third option here will be for you to run the frontend ui tests for this, but they don't exist yet - still on our todo list..)

Copy link
Member

@lkeegan lkeegan left a comment

Choose a reason for hiding this comment

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

lgtm - I merged main and updated all the i18n related stuff, @MaHaWo maybe you could check that I didn't break anything in the process?

@Majapur1
Copy link
Collaborator

@Majapur1 @lkeegan I think this is ready for review now. Could you have a look when you have time? @Majapur1 Can you please check if the deviations from your design are ok and if I overlooked something?\

How can I have a look at it visually on my computer (or on Github)? @lkeegan

The best way imo would be for you to run the whole thing locally - this should be fairly straightforward to do using docker:

https://github.com/ssciwr/mondey/blob/main/DEVELOPMENT.md#run-locally-with-docker

Then you can sign-up as a user on your local version of the website, and whenever you want to see what a branch looks like you can checkout the branch and re-build the docker containers.

If this isn't an option, then an alternative would be for us to merge these changes, update the running deployed website, then you can try it out there and make a new issue if you have any suggestions or changes.

(A third option here will be for you to run the frontend ui tests for this, but they don't exist yet - still on our todo list..)

@lkeegan Thanks for the info. I tried to run the test with playwright, but there was no feedback page, just the main page and registration.

You can merge this, but I still try to run it through docker to be able to check for new changes in the future.

@lkeegan lkeegan merged commit 6de0d9d into main Jan 16, 2025
8 checks passed
@lkeegan lkeegan deleted the incorporate-client-suggestions-for-feedback-page branch January 16, 2025 08:06
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.

Update of the feedback page
4 participants