Feature: Capture notes/comment at the time of Form publish#1546
Feature: Capture notes/comment at the time of Form publish#1546sadiqkhoja wants to merge 2 commits into
Conversation
sadiqkhoja
force-pushed
the
feature/capture-publishing-notes
branch
2 times, most recently
from
888a0f0 to
d3313cb
Compare
sadiqkhoja
force-pushed
the
feature/capture-publishing-notes
branch
from
d3313cb to
eab4a9c
Compare
| <label for="form-draft-publish-note" class="form-label">{{ $t('field.note') }}</label> | ||
| </div> | ||
| <p>{{ $t('introduction[3]') }}</p> | ||
| <!-- We specify two nearly identical .modal-actions, because here we |
There was a problem hiding this comment.
Since notes will always be displayed in the publish modal, I have removed the duplicate modal action buttons.
| "409_6": "The version name of this Draft conflicts with a past version of this Form or a deleted Form. Please use the field below to change it to something new or upload a new Form definition." | ||
| }, | ||
| "field": { | ||
| "note": "Notes (optional)" |
There was a problem hiding this comment.
We don't usually mark form fields as optional. Generally they're optional unless they're specifically marked as required.
Probably more of a question for Nicole, just thought I'd mention it.
| "note": "Notes (optional)" | ||
| }, | ||
| "placeholder": { | ||
| "note": "Add optional publishing notes..." |
There was a problem hiding this comment.
| "note": "Add optional publishing notes..." | |
| "note": "Add optional publishing notes…" |
I think an ellipsis is a little nicer than three periods.
| await modal.setProps({ state: true }); | ||
| modal.find('input').exists().should.be.false; | ||
| modal.findAll('.modal-introduction p').length.should.equal(3); | ||
| modal.find('.modal-introduction').text().should.not.match(/your Draft Form has the same version name/); |
There was a problem hiding this comment.
Was there an issue with the previous assertion? I feel like asserting on the text like this is a little fragile, especially since the assertion uses .not. It'd be easy to modify the wording of the text without realizing that the assertion depends on it. Though I guess this text has been around for a while now.
| if (state) this.versionString = this.formDraft.version; | ||
| if (state) { | ||
| this.versionString = this.formDraft.version; | ||
| this.notes = ''; |
There was a problem hiding this comment.
Could you add a test or modify an existing test to assert that the notes field is reset? There's a related test in form-draft/publish.spec.js for it('resets the input if the modal is hidden'
There was a problem hiding this comment.
How about modifying focusInput() so that it focuses the <textarea> if the <input> isn't rendered? I.e., focusing whichever form control comes first.
sadiqkhoja
force-pushed
the
feature/capture-publishing-notes
branch
from
09c8aac to
be4ac84
Compare
2 participants
Closes getodk/central#1829
Before submitting this PR, please make sure you have:
npm run testandnpm run lintand confirmed all checks still pass OR confirm CircleCI build passes