Skip to content

Add "TagLabel--child" class to aid with custom theming#114

Merged
askvortsov1 merged 10 commits intoflarum:masterfrom
davwheat:patch-2
Feb 1, 2021
Merged

Add "TagLabel--child" class to aid with custom theming#114
askvortsov1 merged 10 commits intoflarum:masterfrom
davwheat:patch-2

Conversation

@davwheat
Copy link
Copy Markdown
Contributor

@davwheat davwheat commented Feb 1, 2021

I've run into issues with theming tags in discussion lists and other areas as there's nothing in the DOM that can be used to differentiate between primary/secondary/child tags.

Adding this custom data attribute would be super helpful for theming, especially with extensions like night mode where keeping the tags the same colour between light/dark could make them unreadable.


PS: The repo seems to be missing the Prettier stuff that other core repos have. Should I PR it all in?

@davwheat davwheat changed the title Add "is-secondary" data attribute to aid with custom theming Add "is-child" data attribute to aid with custom theming Feb 1, 2021
Comment thread js/src/common/helpers/tagLabel.js Outdated
@davwheat davwheat requested a review from SychO9 February 1, 2021 10:34
@dsevillamartin
Copy link
Copy Markdown
Member

Instead of having it equal true or false, I'd rather have it present or not present. Thoughts?

Also - make sure you use single quotes for strings.

@davwheat
Copy link
Copy Markdown
Contributor Author

davwheat commented Feb 1, 2021

Instead of having it equal true or false, I'd rather have it present or not present. Thoughts?

Sounds good. I'll do data-is-child=true and not present, as that seems to match where this sort of stuff is done elsewhere in core.

Also - make sure you use single quotes for strings.

Oops. I'm gonna make a PR to add .prettierrc and the format script because I keep forgetting.

@SychO9
Copy link
Copy Markdown
Contributor

SychO9 commented Feb 1, 2021

why not use a class instead ? TagLabel--child

@dsevillamartin
Copy link
Copy Markdown
Member

Actually, what @SychO9 makes more sense, my bad. We use classes for mostly everything, stuff like if the Post was made by the current user, etc... so TagLabel--child does make more sense.

@davwheat
Copy link
Copy Markdown
Contributor Author

davwheat commented Feb 1, 2021

Ah, that's true. I've always been a fan of data attributes over classes for identifying features like this (generally because they're tstable with ^= or *=, but since classes are used for everything, that's probably a better idea.

@davwheat davwheat changed the title Add "is-child" data attribute to aid with custom theming Add "TagLabel--child" class to aid with custom theming Feb 1, 2021
Comment thread js/src/common/helpers/tagLabel.js Outdated
Comment thread js/src/common/helpers/tagLabel.js Outdated
Comment thread js/src/common/helpers/tagLabel.js Outdated
@askvortsov1 askvortsov1 requested a review from SychO9 February 1, 2021 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants