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.

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

The StringArrayEditor component returns error if you ESC out of it #3462

Open
Gazook89 opened this issue May 7, 2024 · 3 comments
Open

The StringArrayEditor component returns error if you ESC out of it #3462

Gazook89 opened this issue May 7, 2024 · 3 comments
Assignees
Labels
P3 - low priority Obscure tweak or fix for a single user solution found A solution exists; just needs to be applied

Comments

@Gazook89
Copy link
Collaborator

Gazook89 commented May 7, 2024

Renderer

v3

Browser

Chrome

Operating System

MacOS

What happened?

The StringArrayEditor, used for the Tags input, throws an error when you hit esc:

Uncaught TypeError: Cannot set properties of undefined (setting 'editing')
    at ReactClassComponent.closeEditInput (bundle.js:80216:33)
    at ReactClassComponent.handleValueInputKeyDown (bundle.js:80211:12)
    ....

It is this bit:

	closeEditInput : function(index) {
		const valueContext = this.state.valueContext;
		valueContext[index].editing = false;
		this.setState({ valueContext, updateValue: '' });
	},

Code

No response

@5e-Cleric
Copy link
Member

Wow, but if it ain't an obscure bug that could have been laying there for 7 years and we wouldn't have noticed, i'm surprised i didn't find this one.

@5e-Cleric 5e-Cleric added solution found A solution exists; just needs to be applied P3 - low priority Obscure tweak or fix for a single user labels May 7, 2024
@Gazook89
Copy link
Collaborator Author

Gazook89 commented May 7, 2024

I have a fix but not sure it's 100% great yet. But at the same time, I have another question about this:

I would like to set , as a key used to "submit" the current tag and move to the next tag input, the same as the Enter key currently. I don't think this is super controversial, but please let me know if it is.

Further, I am wondering if Tab should do the same. My reflex is that yes, Tab should submit the current tag and move to the next. This would require using Esc to a currently non-existent container around the tags before the user could then use Tab again to move to the next field ("systems"). I am going to look into what specs are relevant for this kind of tag entry. I'm also likely fine with not doing this and just leaving Tab as an option to move to the next field ("systems") without submitting the current tag.

example of tags inside another container:
image

Finally, i think maybe that Tab-ing and Shift + Tab-ing across the tags should activate the 'edit input' state of each tag, rather than sort of skipping them like it does now.

@Gazook89
Copy link
Collaborator Author

Gazook89 commented May 7, 2024

to go further away from original reported issue, i think the stringArrayEditor could be styled a little better to match the Homebrewery UI. For example, get rid of the rounded corners.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 - low priority Obscure tweak or fix for a single user solution found A solution exists; just needs to be applied
Projects
None yet
Development

No branches or pull requests

2 participants