naturalcrit / homebrewery

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

Tweak tags schemas #2333

Open ericscheid opened 2 years ago

ericscheid commented 2 years ago

1. make data-entry of tag prefixes case-insensitive

If someone were to enter Type:Adventure then the current tags data-entry widget flags that input as invalid, which could be confusing. The input should permit case-insensitive text, especially since we're ignoring case when doing searches (e.g. on the user-page).

The submitted tag-type should then be normalised to lowercase. (We use the tag-type to assign a styling class to the tag element on the userpage, so it needs to be standardised there since class names are case sensitive.)

image

Can this be done by just tacking on the i flag at the end of the regex?

https://github.com/naturalcrit/homebrewery/blob/215b64f5a662efdcb3a6205e2c21caea78e44b9d/client/homebrew/editor/metadataEditor/metadataEditor.jsx#L201

<StringArrayEditor label='tags' 
  valuePatterns={[/^(?:(?:group|meta|system|type):)?[a-z0-9][a-z0-9 \/.\-]{0,40}$/i]}

(and also slimming A-Za-z to just a-z since we have the case-insensitive flag in there now)

We should likely also normalise the casing of known tags though to match the datalist options.

2. allow ampersand in tag

image

There will likely be other suggestions for various punctuations from reddit, but at least the & is an obvious candidate.

3. allow non-ASCII characters to support non-English language

image

We should however then disallow various characters, particularly punctuation characters we want to reserve for future use

Or, we run the tag through _.deburr( ) before pattern matching (i.e. dumbify alphabetics).

4. Drop the group: prefix

The group: prefix originated in early discussion of tags, when it was thought we could utilise the tag system for grouping brews into folders on the user-page. The idea of folders has been developed further and using group:tag-string looks to be the wrong direction. We also don't have any coding at all that does grouping of brews on user-pages, so dropping it now doesn't disable anything. We can always add it back if that is the way we want to go.

5. Use <datalist />for standardised options

See #2335 for details — standardisation of terms, discovery of terms, discovery of <category>: syntax.


ericscheid commented 2 years ago

Currently implemented tag categories:

Contenders for additional tag categories:

I have notes on a full list of possible tag values for these. I'll add them to the datalist proposal #2335

Gazook89 commented 2 years ago

Contenders for additional tag categories:

Can you make this a fully inclusive list, not just "additional" categories? So add 'systems', 'type' etc. And as other contributors have additional ideas, edit the comment to include them? I just don't like bouncing around looking at various ideas.

ericscheid commented 1 year ago

Contenders for additional tag categories:

Can you make this a fully inclusive list, not just "additional" categories? So add 'systems', 'type' etc. And as other contributors have additional ideas, edit the comment to include them? I just don't like bouncing around looking at various ideas.

See #2335 for full list.

Gazook89 commented 6 months ago

number 3: allow non-ascii characters

We should however then disallow various characters, particularly punctuation characters we want to reserve for future use

Or, we run the tag through _.deburr( ) before pattern matching (i.e. dumbify alphabetics).

Is this an either/or statement? It seems to me they aren't related--- we can allow non-ascii characters, we can reserve certain punctuation, and we can deburr when do searches (such as on the brew list page).

I just want to double check your thinking here, because I think i've got the non-ascii part figured out with

/^(?:(?:meta|system|type):)?[\p{L}a-z0-9][\p{L}a-z0-9 \/.\-&]{0,40}$/iu

and the allowed punctuation is explicitly set in the regex as well. There is some punctuation in the extended unicode characters, but do we need to reserve those obscure characters?

ericscheid commented 6 months ago

Hmm ... what was I thinking? Good question.

Oh .. I was thinking we could either ..

a) allow any character, but black list specific punctuation we want to reserve, or b) allow non-latin letters (which doesn't include punctuation).

(whichever is easier to write the regex for).

Regardless, deburr should be used.

(Will deburr affect obscure punctuation characters (e.g. ¿?) ? Or just letters?)

Gazook89 commented 5 months ago

regarding number 1:

The input should permit case-insensitive text, especially since we're ignoring case when doing searches (e.g. on the user-page).

The submitted tag-type should then be normalised to lowercase.

I had this all set in PR #3469 and was happy with it, basically running with the suggestion of just using i modifier on the regex to allow any case to be used in the input of the brew tag, and then using a simple function to convert the tag scheme portion to lowercase on submission. So no red "invalid" css styling if you typed Meta: Theme but still converting it to meta: Theme on submission.

But then I realized this component is used for more than one instance-- brew tags and authors-- and that my simple function (cleanValue()) didn't make sense as written to be in the component. It is too specific to one instance, having no application to an instance like Authors. Just for reference, here is that method:

    cleanValue : function(value){
        value = value.trim().replace(/^(.*):/, (match)=>{return match.toLowerCase();});   // convert tag scheme (ex. 'meta:') to lowercase
        return value;
    },

So I'm thinking that maybe stringArrayEditor needs an additional prop-- a transformInput (open to name suggestions). It would accept a function that modifies the submitted input before saving to the components state. Or perhaps the validators prop takes an object or function instead of an array, and a modifying function is optionally included in that?

Lots of thinking out loud here....

Gazook89 commented 5 months ago

So I'm thinking that maybe stringArrayEditor needs an additional prop-- a transformInput (open to name suggestions). It would accept a function that modifies the submitted input before saving to the components state.

I went this route, with modifySubmission as the prop name.