naturalcrit / homebrewery

Create authentic looking D&D homebrews using only markdown
https://homebrewery.naturalcrit.com
MIT License
1.03k stars 316 forks source link

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

Open Gazook89 opened 1 month ago

Gazook89 commented 1 month ago

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 commented 1 month ago

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.

Gazook89 commented 1 month ago

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 commented 1 month ago

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.