Add class attribute as a supported attribute.

[LuisAverhoff]

Currently only the aria-label and access-key attribute are supported. You should probably add class as a supported attribute so that users of this plugin can add a custom class to the shortcut buttons.

// index.ts
....

const supportedAttributes = new Set([
    'accesskey',
    'aria-label',
    'class' // new supported attribute
]);

....

[jcsmorais]

Hi @LuisAverhoff thanks for your suggestion 👍

Do you have a specific use-case in mind where you can't work around it by using, for example, the index of the buttons?

[LuisAverhoff]

@jcsmorais Like for example if I was using a css framework like bootstrap, how would I pass in their css button class since I dont have direct access to the html button element?

You mentioned the index of the button. I'm assuming you are talking about fetching all buttons with the data-set-index and applying the custom css class correct?

I think adding the class attribute as a supported attribute is a much better solution seeing that everyone using this plugin will more than likely want to customize the shortcut buttons and having a quick way to pass the class in versus doing a querySelectorAll for the shortcut button class seems alot cleaner to me.

[jcsmorais]

You mentioned the index of the button. I'm assuming you are talking about fetching all buttons with the data-set-index and applying the custom css class correct?

Yes, I was suggesting something like the following:

.shortcut-buttons-flatpickr-button[data-index="0"] {
  background-color: blue;
}

But I see what you're saying and it does make sense 🙂 Feel free to open a PR and I'll happily review it and get it in an upcoming release 😉

[jcsmorais]

Closing as this request has been fulfilled by https://github.com/jcsmorais/shortcut-buttons-flatpickr/pull/31

[hracik]

@jcsmorais I am using Bootstrap and to style button properly multiple classes are needed, for example: btn btn-sm btn-outline-info

I tried: attributes: { class: ['btn', 'btn-sm', 'btn-outline-info'], } which resulted in class="btn,btn-sm,btn-outline-info" and attributes: { class: 'btn btn-sm btn-outline-info', } which throws error.

Would be great if multiple classes are allowed, probably typed as array which almost works, for example this change works for me, but I don't know how to submit PR.

//this line
//button.classList.add(attributes[key]);
//into these lines
if (typeof attributes[key] === 'string') {
    button.classList.add(attributes[key]);
}
else {
    button.classList.add(...attributes[key]);
}

[larshanskrause]

Why not add your class array by using something like this:

attributes: { class: ['one', 'two', 'three'].join(' ') }

I don't think this plugin should handle the format in which your classes are provided. Or am I missing something?

[hracik]

It does not work.. DOMException: Failed to execute 'add' on 'DOMTokenList': The token provided ('btn btn-sm btn-outline-info') contains HTML space characters, which are not valid in tokens.

Problem is there is probably no way to provide more than one class with current code.

[larshanskrause]

Ah, okay. Quirky classList-add method there. I can add your functionality later today through a PR. But if you can run this plugin code (including the test suite) on your local machine, you could try to make your own PR. It's not that difficult and you already got there most of the way :) It's actually a very good first issue PR and on Google are many resources to teach you how to do it.

[hracik]

I checked how to make a PR, I would be able to do that, but have no idea how to run tests. Would be glad if you can do it, and possibly make a version after it is merged so it can be used with npm|yarn.

[jcsmorais]

I checked how to make a PR, I would be able to do that, but have no idea how to run tests. Would be glad if you can do it, and possibly make a version after it is merged so it can be used with npm|yarn.

Hi @hracik you can run the tests by running the following commands:

yarn build
yarn start:server
yarn test

Your suggestion above looks good 👍
Feel free to submit a PR for it and I'll release a new version once that gets in 😉

[hracik]

Thanks, I tried, but failed. I am not familiar with TypeScript, this line in index.ts is a problem: export type Attributes = { [name: string]: string }; Because I proposed that attribute class can be string or array of strings, but that line expects only string for any attribute. I tried

export type Attributes = { [name: string]: string | string[] };

let attribute: string;
attribute = Array.isArray(attributes[key]) ? ...attributes[key] : attributes[key];
button.setAttribute(key, attribute)

It does not work.. Type 'string | string[]' is not an array type or a string type. Also it would allow other attributes, different from class to be arrays which does not make sense to me.

[hracik]

I managed to create simpler pull request, which does not work with array but with string only.. maybe it is this way even better. {class: 'class1 class2'}

[hracik]

@jcsmorais Is it ok? Can it be merged?