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

💄 [#1817] Show errors for selected documents #858

Merged
merged 4 commits into from
Dec 1, 2023

Conversation

jiromaykin
Copy link
Contributor

@jiromaykin jiromaykin commented Nov 23, 2023

issue: https://taiga.maykinmedia.nl/project/open-inwoner/task/1817

  • change file-list styling when individual files are too large + make info text visible

(File limit can be set in Admin in http://localhost:8000/admin/openzaak/openzaakconfig/ >> Advanced options.

ISBN 111111110 test case: http://127.0.0.1:8000/mijn-aanvragen/2d24e2a4-201c-4803-a8fd-380f071dfdd8/status/

+part of issue: https://taiga.maykinmedia.nl/project/open-inwoner/task/1870

  • make upload button 100% width of container on Desktop
  • decrease bottom-margin in document-lists

@jiromaykin jiromaykin force-pushed the feature/1817-show-errors-for-selected-documents branch from 03c35be to a2a3074 Compare November 23, 2023 11:46
@codecov-commenter
Copy link

codecov-commenter commented Nov 23, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (0269752) 92.80% compared to head (f9cba5d) 92.80%.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #858   +/-   ##
========================================
  Coverage    92.80%   92.80%           
========================================
  Files          798      798           
  Lines        27422    27422           
========================================
  Hits         25449    25449           
  Misses        1973     1973           

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

@jiromaykin jiromaykin force-pushed the feature/1817-show-errors-for-selected-documents branch 5 times, most recently from 1734130 to 8143a46 Compare November 29, 2023 23:06
@jiromaykin jiromaykin marked this pull request as ready for review November 30, 2023 09:02
@jiromaykin jiromaykin requested a review from pi-sigma November 30, 2023 09:18
Copy link
Contributor

@pi-sigma pi-sigma left a comment

Choose a reason for hiding this comment

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

A lingering log statement, otherwise looking good!

src/open_inwoner/js/components/form/FileInput.js Outdated Show resolved Hide resolved
src/open_inwoner/js/components/form/FileInput.js Outdated Show resolved Hide resolved
@jiromaykin jiromaykin force-pushed the feature/1817-show-errors-for-selected-documents branch 2 times, most recently from 733f9be to 14391b0 Compare November 30, 2023 14:50
Copy link
Contributor

@svenvandescheur svenvandescheur left a comment

Choose a reason for hiding this comment

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

On the right track, some remarks about naming convention and coding style though.

src/open_inwoner/js/components/form/FileInput.js Outdated Show resolved Hide resolved
src/open_inwoner/js/components/form/FileInput.js Outdated Show resolved Hide resolved
src/open_inwoner/js/components/form/FileInput.js Outdated Show resolved Hide resolved
src/open_inwoner/js/components/form/FileInput.js Outdated Show resolved Hide resolved
src/open_inwoner/scss/components/Form/FileInput.scss Outdated Show resolved Hide resolved
src/open_inwoner/templates/pages/cases/document_form.html Outdated Show resolved Hide resolved
src/open_inwoner/js/components/form/FileInput.js Outdated Show resolved Hide resolved
@jiromaykin jiromaykin force-pushed the feature/1817-show-errors-for-selected-documents branch from 567cae8 to 087b753 Compare November 30, 2023 15:57
@jiromaykin jiromaykin force-pushed the feature/1817-show-errors-for-selected-documents branch from a0f19e7 to f52e061 Compare November 30, 2023 17:19
@jiromaykin jiromaykin marked this pull request as draft November 30, 2023 20:10
@pi-sigma
Copy link
Contributor

pi-sigma commented Dec 1, 2023

This looks good to me. As far as I can see, Sven's comments have been addressed.
@svenvandescheur Can you take a brief look at this and confirm or disconfirm?

Copy link
Contributor

@svenvandescheur svenvandescheur left a comment

Choose a reason for hiding this comment

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

Small last remarks.

@alextreme
Copy link
Member

Discussed with Paul and Sven because of the remarks and the draft-PR status, and I choose to merge this in due to the pending release. The remaining remarks are small enough that they can be resolved at a later moment.

@alextreme alextreme marked this pull request as ready for review December 1, 2023 15:11
@alextreme alextreme merged commit 087758a into develop Dec 1, 2023
14 checks passed
@alextreme alextreme deleted the feature/1817-show-errors-for-selected-documents branch December 1, 2023 15:12
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.

5 participants