Use data attributes instead of class names to identify html elements

[tobiasdiez]

Instead of using class names, now data attributes are used to identify html elements. In particular, all selectors are reworked to no longer use class names. This makes it possible to use arbitary class names to style the elements, and in particular fixes #738.

[yairEO]

Thank you, but what is the benefit here? This adds another layer of complexity - handling both class and data attributes

I can solve #738, I just chose not to because I don't want people to mess with the inner input element of Tagify.

[tobiasdiez]

I agree that there is a certain overhead, but in my opinion it is definitely worth because of the following reasons:

• It provides a clear separation of data and styling. Using the style to identify the controls and use it for controlling the behavior leads to a rather strong connection between styling and behavior. In fact, one of the purposes of the data attributes is

  data-* attributes allow us to store extra information on standard, semantic HTML elements without other hacks such as non-standard attributes, extra properties on DOM

  (from https://developer.mozilla.org/en-US/docs/Learn/HTML/Howto/Use_data_attributes)
  Using the style class to store information about the type of a control (tag/remove btn/...) or the status of a tag, is such a workaround that should be better handled with data attributes.

• As a result, users of the control can now change the style as they want without any impact on the behavior. This is a big plus in my opinion.

• In particular, one can use multiple style classes without any negative effects. This is, in particular, important if you follow an utility-based approach to css such as tailwind where you usually end up with a rather long list of classes, but is not limited to this use-case of course.

• There is actually not much overhead. The control type attribute is added in the templates and is then no longer changed. So only the tag-status needs to be updated later in the code.

• Should make maintainability of the control easier in the future since you can improve/change say the styling without any need to worry about the functionality since they are now two completely independent things. In short, you get all the advantages that come along with a separation of concerns.

[yairEO]

As a result, users of the control can now change the style as they want without any impact on the behavior. This is a big plus in my opinion.

What do you mean change the style as they want without any impact on the behavior?
You can modify the CSS of tagify selectors (elements) however you want.

The styles are not coupled to the JS logic.. and you can add multiple class names per element (from the Tagify settings object)

[tobiasdiez]

Currently, there is a connection between what you put in in settings.classNames and how the tagify control works (because whatever you put in the class names in order to style the control is used as selectors by tagify). Thus, there is a connection between the styles (class names) and the behavior (selectors). For example, if you add another completely unrelated html element that has the same class as your tagify tag's, then they are treated by tagify as a normal tag. In this way, styles are indeed coupled to the logic/behavior.

You can add multiple class names per element

In my experience, this doesn't work reliably. Indeed, in a lot of places the selector is defined as '.' + settings.classNames.tag but if settings.classNames.tag contains multiple classes, e.g. tagA tagB then the selector ends up being .tagA tagB instead of the correct .tagA.tagB.

[yairEO]

if you add another completely unrelated html element that has the same class as your tagify tag's,

That can never happen because the class names are very unique, unless someone did it intentionally, and therefore they probably know what they are doing.

---

In my experience, this doesn't work reliably. Indeed, in a lot of places the selector is defined as '.' + settings.classNames.tag but if settings.classNames.tag contains multiple classes, e.g. tagA tagB then the selector ends up being .tagA tagB instead of the correct .tagA.tagB.

This isn't a problem, I can only use the first class name as the JS selector.

I initially used custom HTML tag names for the selectors and then switched to classes, but for bulletproofing things, I can switch back to custom HTML tags. I have been doing this for almost 20 years..

For example, this is the tag template (from the templates.js):

<tag title="${(tagData.title || tagData.value)}"
                    contenteditable='false'
                    spellcheck='false'
                    tabIndex="${this.settings.a11y.focusableTags ? 0 : -1}"
                    class="${this.settings.classNames.tag} ${tagData.class ? tagData.class : ""}"
                    ${this.getAttributes(tagData)}>
            <x title='' class="${this.settings.classNames.tagX}" role='button' aria-label='remove tag'></x>
            <div>
                <tagText class="${this.settings.classNames.tagText}">${tagData[this.settings.tagTextProp] || tagData.value}</tagText>
            </div>
</tag>

As you can see, these are the Tagify-only tags: <tag>, <x>, <tagText> (just added tagText now for example)

For class names which are add/removed only by Tagify - they should stay as-is (as classes)

Anyway, this and your PR are breaking-changes and can affect people that already use class names to style their tagify components. I would have to have to write a warning in the README for this specific change.

[tobiasdiez]

That can never happen because the class names are very unique, unless someone did it intentionally, and therefore they probably know what they are doing.

Fair enough. This was meant more as an example to demonstrate the connection.

This isn't a problem, I can only use the first class name as the JS selector.

Currently, it's a problem. Using the first class name might work, based on context. For example, if one sets settings.className.tag = "p-3 justify center-align" or something equivalent in say tailwind, then "p-3" would be the selector which is probably not what you want. But if you introduce conventions that would require to have "tagify-tag" as the first class, then this approach would work. Not sure if it's really simpler than the approach via data attributes.

I can switch back to custom HTML tags

If I may, I would advice not to use custom html tags for this purpose. Data attributes were developed especially for the purpose to provide a standard and well-recognized way to implement such meta-data into html elements. I think it would also not help with differentiating between different status of tags (editing, hiding, ...).

Anyway, this and your PR are breaking-changes and can affect people that already use class names to style their tagify components.

I think you misunderstood my PR then somewhat. I didn't change anything concerning the styles. In particular, all css classes are still applied as they are currently are, so existing users of the control don't need to change anything. It's a purely under-the-hood change, about how different states and controls are identified and thus should be completely backwards compatible.

[yairEO]

I see, I now better understand the intentions, for bulletproofing the selectors in the JS logic to, but Tagify also relies heavily on CSS so a developer should not do:

settings.className.tag = "p-3 justify center-align"

Because it will be missing the class name of the tag and thus break things visually, which then makes the component useless...

I can understand why developers might want to add class names but cannot think of why would they completely remove the default ones..so I always expected developers do only add, keeping the default name because they obviously understand it's important to keep and should always be the first in the list of class names:

                               👇
settings.className.tag = "tagify__tag p-3 justify center-align"

So I understand you want to de-couple these from the JS logic, so JS won't break, but the UI will break anyway without tagify__tag class (in the above example), so what's point?

I don't think anyone would completely re-write the CSS so the tagify__tag class name wouldn't be required any more

---

I can add a section in the README to explicitly explain this whole thing

[tobiasdiez]

I don't think anyone would completely re-write the CSS so the tagify__tag class name wouldn't be required any more

That's actually my intention 😄. I started with this and then I discovered the current limitations when it comes to multiple classes, which lead me to this PR. Especially when integrating with popular CSS frameworks (such as Bootstrap, tailwind etc), quite a few changes to the css are necessarily and it is easier to start from 0.

I agree that carrying for example "tagify__tag" around (and if it's only as a dummy class) would work in most use cases. But I do think that separating styling from the selectors used in the javascript code is advantageous and preferable over some conventions of css classes.

[yairEO]

yeah makes sense.
I will look into this in the upcoming days, as I have something else very urgent to code for the next few days

[tobiasdiez]

@yairEO did you already found some time to look at this PR?

[yairEO]

Sorry... had some real-life-stuff wasting my time instead of coding.. I was supposed to go over this in the weekend but didn't get to it. This is high on my to-do list, so don't worry.

Tagify is my hobby, along with dozens other projects on Github and I devote my time between them all

[yairEO]

Why did you choose data attributes over id?

[tobiasdiez]

Because there could be for example multiple tags, so id = tag wouldn't work. Moreover, the information that needs to be stored is some metadata about the corresponding control, and that's the purpose of the data attributes.

[yairEO]

Ho, THAT limitation :D

[tobiasdiez]

@yairEO ping 😉

[yairEO]

@tobiasdiez - hi, I was working hard on other open-source projects of mine since Tagify is already doing very well, so I only had time for bugfixes and not large changes such as this. Is this something which is urgent to you?

[tobiasdiez]

@yairEO Don't worry. I'm maintaining a few open-source projects myself, so I know that time is a precious resource.

I wouldn't say it's urgent, but nonetheless it would be nice to have this PR merged. It's a nice additional feature that users can now specify arbitrary style classes. Moreover, the longer this PR is open the larger the chance is that changes to the main branch are incompatible, possibly leading to bugs etc.

[tmaier]

This is a great idea! Going for data attributes is also what some frameworks propose to do more, e.g. StimulusJS

[yairEO]

I've started, will do more work on this soon.

[HybridSolutions]

How is this going? Seems the project is not updated anymore. It was great!

[yairEO]

Yeah it's not really getting updates because of my time going to other things..

I have a billion open tabs on things to do or read and it seems I can barely keep up,
as the world is going forward faster and faster and I constantly must read more and more
and do less and less actual coding..

This project isn't dead - I really want to get some things done.
the codebase is far far from my current coding standards as I've started it in 2017 as
a small project that bloated in size due to endless features, and not it gotten a bit
more difficult to maintain

[HybridSolutions]

Yeah it's not really getting updates because of my time going to other things..

I have a billion open tabs on things to do or read and it seems I can barely keep up, as the world is going forward faster and faster and I constantly must read more and more and do less and less actual coding..

This project isn't dead - I really want to get some things done. the codebase is far far from my current coding standards as I've started it in 2017 as a small project that bloated in size due to endless features, and not it gotten a bit more difficult to maintain

Yes, I understand what you mean perfectly. Still working great so that's ok. Still is one of the best solutions out there for tags ;)