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

emit tag classname as lowercase on userpage #3166

Open
ericscheid opened this issue Dec 3, 2023 · 5 comments
Open

emit tag classname as lowercase on userpage #3166

ericscheid opened this issue Dec 3, 2023 · 5 comments
Labels
P2 - minor feature or not urgent Minor bugs or less-popular features solution found A solution exists; just needs to be applied tweak Small, non-breaking change

Comments

@ericscheid
Copy link
Collaborator

Tags can be entered when enditing a brew as <tag-type>:<tag-name>, where <tag-type> is a limited enumerated set of values. That tag data-entry UI also enforces lowercase for <tag-type>.

It is possible however to circumvent that UI (e.g. edit the google-doc source).

The <tag-type> is emitted on the userpage by setting the className of the tag display element: <span class="system">C&C</span>. This is so different tag-types can be styled differently (e.g. "system" tags in blue, untyped tags as yellow).

However, css classnames are case sensitive. We should enforce the schema design constraint not only in the data-entry UI, but also in the emitting of tags onto the userpage. (Postel's Law)

The relevant code to fix is on line 132 here:

{brew.tags?.length ? <>
<div className='brewTags' title={`Tags:\n${brew.tags.join('\n')}`}>
<i className='fas fa-tags'/>
{brew.tags.map((tag, idx)=>{
const matches = tag.match(/^(?:([^:]+):)?([^:]+)$/);
return <span key={idx} className={matches[1]}>{matches[2]}</span>;
})}
</div>

@ericscheid ericscheid added solution found A solution exists; just needs to be applied tweak Small, non-breaking change P2 - minor feature or not urgent Minor bugs or less-popular features labels Dec 3, 2023
@ericscheid
Copy link
Collaborator Author

Philosophically, there should also be an intervention in some middleware to "correct" data errors on saving/loading. That's more complicated though.

@G-Ambatte
Copy link
Collaborator

return <span key={idx} className={matches[1]?.toLowerCase()} onClick={()=>{this.updateFilter('tag', tag);}}>{matches[2]}</span>;

@5e-Cleric
Copy link
Member

Another point is that tags are not trimmed before being set, so a !== a

@ericscheid
Copy link
Collaborator Author

While stray spaces on tags do cause problems elswhere, they don't cause a problem with class names as class=" system" is == to class=" system" as far as html/css cares.

@5e-Cleric
Copy link
Member

While stray spaces on tags do cause problems elswhere, they don't cause a problem with class names as class=" system" is == to class=" system" as far as html/css cares.

I know, just saying we should trim when inputing tags

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 - minor feature or not urgent Minor bugs or less-popular features solution found A solution exists; just needs to be applied tweak Small, non-breaking change
Projects
None yet
Development

No branches or pull requests

3 participants