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

Tweak tags schemas #2333

Open
6 tasks
ericscheid opened this issue Sep 4, 2022 · 7 comments
Open
6 tasks

Tweak tags schemas #2333

ericscheid opened this issue Sep 4, 2022 · 7 comments
Assignees
Labels
Approved Has been discussed and an approach is agreed upon P2 - minor feature or not urgent Minor bugs or less-popular features

Comments

@ericscheid
Copy link
Collaborator

ericscheid commented Sep 4, 2022

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?

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

<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.


  • make tag prefixes case-insensitive
  • normalise casing of known tags
  • allow ampersand in tag
  • allow non-ASCII characters to support non-English language
  • remove group: prefix
  • use <datalist /> for standardised options
@ericscheid ericscheid changed the title Tweak tags Tweak tags entry UI Sep 5, 2022
@ericscheid ericscheid changed the title Tweak tags entry UI Tweak tags schemas Sep 5, 2022
@calculuschild calculuschild added Approved Has been discussed and an approach is agreed upon P2 - minor feature or not urgent Minor bugs or less-popular features labels Sep 5, 2022
@ericscheid
Copy link
Collaborator Author

ericscheid commented Sep 27, 2022

Currently implemented tag categories:

  • system: e.g. D&D 2e, WFRP 5e
  • type: e.g. Adventure, Campaign, Monster, Class
  • meta: e.g. Guide, Theme, Template
  • group: — deprecated

Contenders for additional tag categories:

  • genre: e.g. horror, gothic, high fantasy, grimdark, mystery, romance
  • license: e.g. OGL, PD, CC BY SA

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

@Gazook89
Copy link
Collaborator

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
Copy link
Collaborator Author

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 Gazook89 self-assigned this May 9, 2024
@Gazook89
Copy link
Collaborator

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
Copy link
Collaborator Author

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
Copy link
Collaborator

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
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Has been discussed and an approach is agreed upon P2 - minor feature or not urgent Minor bugs or less-popular features
Projects
None yet
Development

No branches or pull requests

3 participants