-
Notifications
You must be signed in to change notification settings - Fork 100
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
feat: Add forms state
to close and archive forms
#1925
Conversation
5ea7066
to
3e64873
Compare
state
to close and archive formsstate
to close and archive forms
b7602c3
to
4215fbe
Compare
4215fbe
to
25395ab
Compare
Rebased, squashed and fixed the tests. Ready for review :) |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
25395ab
to
ba2118d
Compare
This comment was marked as resolved.
This comment was marked as resolved.
b7b3a5c
to
20124f7
Compare
I have added screenshots and requested a review from @jancborchardt :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, "Archived forms" would be better as a sticky entry on the bottom left which opens a modal, just like in Contacts and Calendar.
20124f7
to
2ffdaaf
Compare
Thank you @jancborchardt ! vokoscreenNG-2024-02-19_21-36-15.mp4 |
2ffdaaf
to
6555d2c
Compare
Done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good now :)
Just some design/UX related things: Should we make it possible to dearchive a form directly from the modal? And on a public share page you always show the "expired" message in the NcEmptyContent as the FormsService always returns isExpired as soon as the state is not "active". So no hint for archived/closed there...
One more comment: the |
Please integrate the fix for #1991 into this PR. Should be easy to fix and you can directly address the new state prop as well :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the late design feedback, 3 things:
- The layout of the left navigation is a bit off, lot of whitespace (e.g. below the "Your forms" heading) and the entries are too big. Since there is no subline, they should be single-line entries like in Files or the left navigation of Mail, not multiline list items like in Talk or the message list in Mail.
- The "Archived forms" entry on the bottom does not have proper spacing to the bottom. Check how "Files settings" does it – left aligned, tertiary button, non-bold text.
- In the modal, the actions button should show for all entries by default, not only on hover or focus → this is better for discoverability
This is only true for forms without expiration date. Then the expiration is shown in the subline. Also closed forms show the information in the subline. So no changes needed here :) |
6555d2c
to
262666b
Compare
Thank you for the feedback, I resolved the comments :) a.mp4 |
17f34a1
to
4c4b77d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Chartman123 changes make sense, one comment.
But I think we need a follow up some when, because always loading all forms is pretty bad. For small instances this works but what if there are 30'000 forms? 100'000?
Archived forms can not be changed (except from being un-archived). Closed forms behave like expired forms and just do not allow new submissions. By default forms are in state `active`. Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
4c4b77d
to
1c91ebf
Compare
Should be ready to merge now |
1c91ebf
to
16c265c
Compare
Co-authored-by: Chartman123 <chris-hartmann@gmx.de> Co-authored-by: Ferdinand Thiessen <opensource@fthiessen.de> Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
16c265c
to
8660fce
Compare
I recommend to hide white space when review.
This adds a new field "state" to a form, this field is
0
by default which means the form is in active state.Currently two other states are implemented:
closed
when then form was manually closedarchived
when the form was archivedClosed forms do not allow any new submissions, archived forms even do not allow any modification.
Archived forms are shown in a separate list on the side bar to save space.
Screenshots