flarum / issue-archive

0 stars 0 forks source link

Add relevant CSS classes to stickied, locked, and maybe subscribed discussions #215

Open askvortsov1 opened 4 years ago

askvortsov1 commented 4 years ago

Feature Request

Is your feature request related to a problem? Please describe. It's difficult for forum admins to add custom CSS to discussions that are stickied only, such as a highlighted background. The same applies to locked and maybe subscribed discussions. Relevant: https://discuss.flarum.org/d/23486-is-it-possible-to-change-background-color-of-sticky-post

Describe the solution you'd like A 'discussion-sticky' (or similar) css class should be added to sticked discussions, and the same should be done for locked and subscribed ones.

Justify why this feature belongs in Flarum's core, rather than in a third-party extension This will allow forum admins to easily add custom CSS to sticky discussions only.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We do this to keep the amount of open issues to a manageable minimum. In any case, thanks for taking an interest in this software and contributing by opening the issue in the first place!

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We do this to keep the amount of open issues to a manageable minimum. In any case, thanks for taking an interest in this software and contributing by opening the issue in the first place!

stale[bot] commented 3 years ago

We are closing this issue as it seems to have grown stale. If you still encounter this problem with the latest version, feel free to re-open it.

SychO9 commented 3 years ago

Not an urgent issue, but great to have to improve the theming experience.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We do this to keep the amount of open issues to a manageable minimum. In any case, thanks for taking an interest in this software and contributing by opening the issue in the first place!

davwheat commented 3 years ago

Maybe we should consider using data attributes for this instead?

data-discussion-type="sticky subscribed" would allow for CSS to use [data-discussion-type*="subscribed"] to match all subscribed discussions?

SychO9 commented 3 years ago

Since the purpose is styling, modifier class names are better and more consistent with the code base.

davwheat commented 3 years ago

I'm aware that it's more consistent, but I think that at some point we should start moving towards data attributes.

Attributes like these are more commonly used for storing data related to the component/element, which I think is a perfect fit for this situation.

We already use data-type, data-number, etc on post stream items. Why is that so different to this?

<div class="Discussion Discussion--stickied Discussion--locked">

<div class="Discussion" data-modifiers="stickied locked">

To me both of these are equally readable.

Data attributes also make it much easier to interact with the element in JS, in my opinion, with the element.dataset object.

SychO9 commented 3 years ago

Attributes like these are more commonly used for storing data related to the component/element, which I think is a perfect fit for this situation.

We already use data-type, data-number, etc on post stream items. Why is that so different to this?

What's different is those were not introduced for the purpose of styling, but for JS manipulation. And there doesn't seem to be a valid reason for why we should drop modifier class names in favor of data attributes and therefore introduce more overly specified selectors in our styles.

Here's another proposition instead, we can introduce data attributes, while still use modifier class names for CSS selectors.

tldr; I really like the convention we're following and would rather we stick to it (https://github.com/suitcss/suit/tree/master/doc).

(Also ref previous discussion of this: https://github.com/flarum/core/pull/2797#discussion_r616759240)