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.

Sign up for GitHub

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

Jump to bottom

TagInput - Functional component for tag-like inputs #3735

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open

TagInput - Functional component for tag-like inputs #3735

wants to merge 20 commits into from
+113 −155

Conversation

Gazook89
Copy link
Collaborator

@Gazook89 Gazook89 commented Sep 19, 2024 •
edited
Loading

Description

This supersedes PR #3469. This PR is just the very basic functionality of tags: add, remove, update. Subsequent PRs will layer on additional features/functionality in discrete chunks and descriptions are below. Each of the additional features is a branch on the previous PR-- they build on each other. As such, they can merged in order, or the last PR can be merged in and it will contain everything prior to it. Here is the breakdown:

#3735 - Part 1 - Basic Functions The PR removes the current StringArrayEditor.jsx entirely, and replaces it with tagInput.jsx which creates a functional component called TagInput. Obviously it doesn't have to be limited to only "tags" as we called them in HB-- it can be used for the Author list as well and where you might want a *string array editor*. I just prefer the shorter name for brevity.
  • add tags
  • remove tags individually
  • edit existing tags
  • save tags to brew metadata
[No PR yet] - Part 2 - Keyboard Navigation

Part 2 focuses on adding keyboard navigation to the tag input. Things like using Enter, ,, or Tab to submit a new value, or tabbing/shift+tabbing to move around the tags.

Here is the branch on my remote.

[No PR yet] - Part 3 - Validations

Part 3 adds methods to validate the tag inputs so that we can control what values are input. It differs from the original StringArrayEditor by having the validator functions centralized with other inputs in validations.js, rather than declaring the functions directly in props or as valuePatterns.

Here is the branch on my remote.

[No PR yet] - Part 4 - Uniqueness

Part 4 is basically a supplement to Part 3-- it adds a check to see if the new value is unique from the existing tags before submitting it. By default, it must be unique, but via props the component could accept duplicate values.

Here is the branch on my remote.

Quick Overview of TagInput

Click to expand

The component is called by the parent, MetadataEditor, which passes in an array of strings which represent the values of each tag (tag can be any data type, like Tags or Authors). Additional props can be passed in, such as validation regexes, a boolean "unique?" prop, "non-editable" tags, etc-- though right now, most of those features aren't in this PR yet.

With the array of strings, TagInput creates a new array of objects. Each object has two properties: value and editing. The latter is set to false by default. Each tag is rendered as a list of divs that are "read only", or in the code, renderReadTag(). These elements have a click event that changes the editing property of the tag to true, and converts them to "write" tags (renderWriteTag()), which are text inputs that allow you to edit the tag.

TagInput passes an array of strings back up to MetadataEditor which the processes it with it's own handleFieldChange handler.

Related Issues or Discussions

 

QA Instructions, Screenshots, Recordings

This is the visual design for THIS PR, Part 1:
image

For reference, here is a screenshot from Part 4:
image

Reviewer Checklist

*Reviewers, refer to this list when testing features, or suggest new items *

  • Verify new features are functional
    • add new tags, remove existing tags, edit existing tags
    • changes to tags are saved to brew, and are rendered on reload
    • loads existing tags (from brews that existed prior to this PR)
  • Test for edge cases / try to break things
  • Identify opportunities for simplification and refactoring
  • Check for code legibility and appropriate comments
Copy this list 
- [ ] Verify new features are functional
	- [ ] add new tags, remove existing tags, edit existing tags
	- [ ] changes to tags are saved to brew, and are rendered on reload
	- [ ] loads existing tags (from brews that existed prior to this PR)
- [ ] Test for edge cases / try to break things
- [ ] Identify opportunities for simplification and refactoring
- [ ] Check for code legibility and appropriate comments

@Gazook89
Initial commit: Rename component, set basic structure
e8e16f4 
No actual functionality implemented yet, just renames the component from "StringArrayEditor" to "TagInput", for brevity at the possible cost of clarity.  For now, the original StringArrayEditor is kept and named "TagInput-class.jsx" so that I can reference it as I work on the functional component.
@Gazook89
Merge branch 'master' into Functional-Tag-Editor
ea7f18e
@Gazook89
Render element for each value from props
d505e4e 
Take an array of values from props, load it into valueContext state with an "editing" boolean for each value.  Then, when rendering the component, take each value in the valueContext array and create a div for each --

at this point, if the value is "being edited", it returns a div with text "editing".  If not being edited, it returns a div with the value as text.

Nothing is being edited at this point since that functionality doesn't exist yet.
@Gazook89
Render tags as "write" or "read"
d5c5b43 
Tags are now either "readTag" or "writeTag", with the former being a div with the tag value and the latter a text input with the value.

Minor class name change in LESS.
@Gazook89
Add click handler for readTags to open text input
36aa4ea 
Clicking on a readTag now converts that tag to a text input, and maintains the tag value.  It also closes any other open text inputs amongst the tags (but leaves the "new tag" input open).
@Gazook89
add editing of input functionality
c5033db 
Currently uses uncontrolled inputs with a `defaultValue` attribute set to the values passed in via props.  The input can then be edited, and when `Enter` is pressed, it updates the stored value state.  Later, this can be updated to be trigger with `Tab` or clicking outside the active input element.
@Gazook89
Add in handlers for TagInput value changes
d1686c4 
Now brew metadata is actually updated and preserved across reloads to match updated tag values.  useEffect calls the props.onChange event from the parent component on every change to the valueContext state of this component (right now, after hitting Enter in a tag input).
@Gazook89
Add method for adding new tags
70a3cb9 
Component now accepts new tags entered in the always-present input field.  Entering a value and hitting Enter submits the tag, and it appears as a new tag.

Updated the tag list keys to be unique (via `index`).

To-Do: empty 'new tag' input after submitting.
@Gazook89
Add 'remove' button and method
c65210b 
New button that triggers `submitTag()` method directly (rather than throw onKeyDown event) and passes `null` as the newValue.  New `if` condition checks for null on newValue and if true, removes the tag that matches the originalValue.

This *does* currently delete all duplicate tags if they match the one you are deleting.  Not sure when you'd ever want duplicate tags, but regardless i'll likely switch this to work via Index, not value.
@Gazook89
Use index to find and remove tags
c1288ce 
Fixes issue in last commit, so removing a tag that has duplicate value of other tags only removes the correct one, not the others as well.
@Gazook89
Set new tag input to clear itself after submission
a54adc1 
Now whenever a new tag is submitted, the input element is cleared and ready for the next tag.

Whitespace cleanup.
@Gazook89
Add comma to "submit" buttons
5b4a7c1 
Now comma (`,`) submits a tag, like `Enter`
@5e-Cleric
Copy link
Member

Updated initial comment to actually have the list to copy

@Gazook89
Adjust html structure to handle tags as list
7ea1696 
Begin work on setting a better html structure for the component.

Create a .less file for the component, which I may not actually use.
@Gazook89
Merge branch 'Functional-Tag-Editor' into Func-Tag-Editor-Features
8a67e1e
@Gazook89
Fix multiple duplicate tags updating at once
b585e85 
Fixes an issue where tags with duplicate values would all update to the same value after editing just one.

Also an adjustment to the parameters that are passed to handleInputKeyDown-- they are now one object.  This helps handle an "options" object where more optional features can be turned on and off.
@Gazook89
Merge branch 'master' into Functional-Tag-Editor
a9b6d5f
@Gazook89
remove tagInput-class
5af45f1 
This file was just the old StringArrayEditor that I kept around for easy reference.  Can be deleted now.
calculuschild
calculuschild approved these changes Sep 23, 2024
View reviewed changes
Copy link
Member

@calculuschild calculuschild left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This seems functional and clean enough to me for a start. Just some variable naming suggestions.

Just nobody merge this yet until I can deploy the latest master branch live.

  • Verify new features are functional
    • add new tags, remove existing tags, edit existing tags
    • changes to tags are saved to brew, and are rendered on reload
    • loads existing tags (from brews that existed prior to this PR)
  • Test for edge cases / try to break things
  • Identify opportunities for simplification and refactoring
  • Check for code legibility and appropriate comments

@Gazook89
Variable name changes for clarity
a96ff6e 
Followed suggestions on the PR.
@5e-Cleric 5e-Cleric added UI/UX User Interface, user experience 🔍 R4 - Reviewed - Fixed! Awaiting final review⭐ PR review comments addressed labels Oct 23, 2024
@Gazook89 Gazook89 mentioned this pull request Oct 24, 2024
Draft
3 tasks
@5e-Cleric 5e-Cleric marked this pull request as ready for review October 28, 2024 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@5e-Cleric 5e-Cleric 5e-Cleric left review comments

@calculuschild calculuschild calculuschild approved these changes

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
🔍 R4 - Reviewed - Fixed! Awaiting final review⭐ PR review comments addressed UI/UX User Interface, user experience
Projects
@calculuschild's backlog
Status: Pushing to Finish
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Gazook89 @5e-Cleric @calculuschild