Added free keyword text area#694
Conversation
✅ Deploy Preview for dailp ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
alisony755
left a comment
alisony755
left a comment
There was a problem hiding this comment.
This looks good and works well! I like the option to add either an existing keyword or a custom one.
| onRemove?: (index: number) => void | ||
| addButtonLabel: string | ||
| customForm?: React.ReactNode | ||
| children?: React.ReactNode |
There was a problem hiding this comment.
It might be helpful to be slightly more clear about the purpose of children, either with a comment or renaming it to something like freeInput.
There was a problem hiding this comment.
Or additionalForm like you use for subject headings
alisony755
dismissed
their stale review
I'm not sure if they're supposed to, but the keyword changes (as well as the changes to the other metadata fields) don't persist after the modal is submitted?
GracefulLemming
left a comment
GracefulLemming
left a comment
There was a problem hiding this comment.
No blocking comments here, just some food for thought.
| value={freeKeyword} | ||
| onChange={(e) => setFreeKeyword(e.target.value)} | ||
| onKeyDown={(e) => { | ||
| if (e.key === "Enter" && freeKeyword.trim()) { |
There was a problem hiding this comment.
I like this shortcut, but it does have overlap with the submit button which may be disorienting for users. You don't need to change this PR but I think the solution could involve the submit button only working on focus (assuming that solution doesn't run us astray of accessibility compliance).
@chullings and @jae-mess we should think more about keyboard use across the site
There was a problem hiding this comment.
I removed the shortcut as while nice, I didn't see that conflict and think that it is best to leave it up to site-wide decisions (especially since the other fields don't have it).
| /> | ||
| addButtonLabel="Add Pre-existing Keywords" | ||
| > | ||
| {isEditing && ( |
There was a problem hiding this comment.
@chullings are there any metadata fields that shouldn't allow extension from editors?
If we can tolerate all metadata fields drawing from user-built dictionaries like keywords does here, then I think we can move the responsibilities within this block into the TagSelector component
There was a problem hiding this comment.
You're technically right. At the time of writing this code, I believe that contributors were allowed to edit metadata but only editors were allowed to add new keywords. This check is redundant now since only editors can write into metadata.
| addButtonLabel="Add Pre-existing Keywords" | ||
| > | ||
| {isEditing && ( | ||
| <div |
There was a problem hiding this comment.
Do we need two input fields?
"Add Keyword" and "Select Pre-existing Keyword" both interface with a dictionary of keywords (stored in the keyword table). Why not collapse these fields like we do with word parts?
So, when a user types into the input field, they receive hints pulled from the dictionary (using a substring match). A user can add an existing keyword to a document's metadata by selecting a keyword from the hint list or typing out a keyword in full that is already present in the dictionary. A user can add a keyword that is not in the dictionary by typing it in full before pressing "Add Keyword" (or pressing enter in this case); In this case, the keyword would be added to both the document metadata and the dictionary.
There was a problem hiding this comment.
As for this pr, I think this is fine by the specs. In the future I believe all fields will work as you've just described as I've done this with later work for the subject headings.
3 participants
Allows for users to input a maximum of 40 characters for a keyword when editing meta data. Additionally, kept the selector for preexisting keywords for easy of use.