search

open-formulieren / open-forms-sdk

A Javascript SDK for Open Forms
https://open-forms.readthedocs.io/en/stable/developers/sdk/index.html
Other
2 stars 6 forks source link

[OF#180] Add GovMetric #634

Closed SilviaAmAm closed 7 months ago

SilviaAmAm commented 8 months ago

Fixes open-formulieren/open-forms#180

Backend: https://github.com/open-formulieren/open-forms/pull/3775

codecov[bot] commented 8 months ago

Codecov Report

Attention: 12 lines in your changes are missing coverage. Please review.

Comparison is base (42b47c4) 73.04% compared to head (8845f0b) 75.12%. Report is 31 commits behind head on main.

Files Patch % Lines
src/components/AbortionButton/AbortionButton.js 70.00% 6 Missing :warning:
src/components/ExistingSubmissionOptions.js 57.14% 3 Missing :warning:
src/components/Form.js 71.42% 2 Missing :warning:
src/components/FormStart/index.js 50.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #634 +/- ## ========================================== + Coverage 73.04% 75.12% +2.07% ========================================== Files 219 224 +5 Lines 4474 4542 +68 Branches 1186 1211 +25 ========================================== + Hits 3268 3412 +144 + Misses 1167 1091 -76 Partials 39 39 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

SilviaAmAm commented 7 months ago

The button seems to appear on the right in the stories:

It is because the AbortionButton component returns a toolbar. Maybe this doesn't make much sense and it should return a button and then the component using it can add the toolbar :thinking:

Apart from that, if I understand correctly this also simplifies the current onFormAbort / onLogout logic and everything is centralized in one place?

Yes indeed! I tried combining those into one place. There is still the component taking care of saving the submission halfway through that also uses similar code, but I was worried of breaking it, so I didn't refactor it for now.

SilviaAmAm commented 7 months ago

Extra thought: maybe we should always have an abortion button, also if govmetrics isn't enabled :thinking: Then we could refactor ExistingSubmissionOptions further

Viicos commented 7 months ago

Extra thought: maybe we should always have an abortion button, also if govmetrics isn't enabled 🤔 Then we could refactor ExistingSubmissionOptions further

I think it could be great, as a user I would expect an explicit way to abort, without having to close the browser and wonder about what actually happens when I do so

SilviaAmAm commented 7 months ago

Follow up ticket: https://github.com/open-formulieren/open-forms/issues/3791

sergei-maertens commented 7 months ago

Please see the comments on the Chromatic build - there's some empty blank space now that should ideally not be rendered if the abortion button is not relevant.

sergei-maertens commented 7 months ago

Also merged the main branch in stable/2.5.x now.