-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add support for array style options #15
Comments
Well… But yeah, multiple options in a kind of multi-select input would be great. However, the UI is totally missing here. Apart from that, I don't consider it out-of-scope, it's just hard to implement technically and I never really needed it and could find other solutions, so it's not in there. |
Yeah, it could require some significant JS to implement the UI.
I have at least three extension ideas which would need this feature, however I think the four other issues I created are a higher priority, especially the two blocking parts of Unicodify and the Awesome Emoji Picker, although I would definitely be interested to hear how you think we could best implement this feature... Back to my weather extension example, I was thinking developers could specify their default settings like this to indicate that they want to use array style options for an option group: const defaultSettings = {
locations: [
{
zip: "20500",
country: "US"
}
]
}; |
For adding more than one instance of an option group, I implemented a simple UI using a table, which has a column for each option in the option group and a row for each instance of the option group: https://jsfiddle.net/4bafm35n/. There is also a button to convert the table to an array, to demonstrate what would be needed by this library to save the values in the table. This library would then just need run that code anytime the user modified any of the cells. For the select elements where the
I would also be happy to discuss how we could best update the library to support this feature... |
What you read sounds all goods far as for what you write. A table sounds reasonable as a HTML markup for that, though I maybe would use that in the code, but rather use classes, so you can use your own markup there (after all, maybe the options also are not supposed to be in one line/could spawn multiple lines). So maybe it could be designed similar to how the options groups are implemented. Instead of a string as a thing to copy for each entry/"line"/"row", it should use a HTML . Maybe also a slot for where this stuff, but I am not sure how this WebComponent thing is used. Finally, it just needs a button with another class that is used as the "add another entry" button. |
Yes, anyone using your library would need to write their own HTML for the table. That was just to demo a possible UI for this, so that we could make the necessary changes to the library. Specifically, it was using the same weather extension example from my above post.
Yes, I think the HTML Template element would work great here, as the user could then specify their format for each row directly in the HTML file.
I am not sure what you mean by "class". A JS class? A CSS class? I updated my demo to support removing all of the added entries/rows, as well as moving individual rows up and down: https://jsfiddle.net/4bafm35n/1/. I think this is everything most extensions would need. I also used emojis to reduce the amount of text that would need to be localized. Here is another updated version of the demo using the HTML Template element as you suggested: https://jsfiddle.net/4bafm35n/3/. I believe this is all of your suggested changes. It also now ignores any rows with invalid cells when creating the array, using the method suggested in #14. |
The CSS one.
Ah indeed a good idea.
Nice one. |
Similar to #13, I think this could be implemented using the
If I understand you correctly, you are proposing we add a CSS class to each of the buttons. Note that extensions could have multiple options where they need to have more than one instance of an options group and thus they would need multiple tables. This library would then need to know which buttons corresponded to which table. At minimum, the library would need a reference to the table, template for each row, add entry/row button and remove all entries/rows button. Anyway, I believe I implemented all your suggested changes in my demo, although I am not sure I understand what you are proposing we update in your library. Note that this issue is requesting support for two similar features:
My demo was only a possible UI for the former, since you said:
My demo just has simple text inputs in each row of the table, but extensions could obviously have any arbitrary form elements. Edit: Here is another updated version of the demo, which adds a default row/entry to the table and fixes a bug with the buttons: https://jsfiddle.net/2uhb0ryw/ |
Yes, that's easy though. One can get it as this Yeah as for how exactly to implement it I am also not sure, I'd need to look into the lib too anyway. I did not have a look there since quite some time. 🙃 |
I was finally able to integrate my demo with the It actually has five such options that need more than once instance of an options group for each of the five supported issue trackers. Anyway, I found a few issues with this library that I had to work around:
I would be happy to share my code if that would be helpful. |
That all sounds very reasonable and although I don't have that code in mind in detail, I guess yeah… we could change that. Sure sharing your code never hurts (it hopefully lives in some GitHub repo in a branch anyway 😉). Of course, best would be to split this issue into smaller ones, and fix them in this lib one by one then… |
Yes, I had not pushed it yet, but here it is now: https://github.com/tdulcet/Bug-Opener/blob/f250b97540093f103a9a4dcd32c9141f9ec06da0/options/modules/CustomOptionTriggers.js#L19-L120 |
This may be out of scope for this library, but it would be very useful if it supported array style options, where the user can add more than one value to an option or more than one instance of an option group. For example, consider a standard weather app/extension where the user can add multiple locations, each with a ZIP code and country. This would also be needed for any input or select elements where the
multiple
attribute is set.This is currently blocking rugk/unicodify#30.
The text was updated successfully, but these errors were encountered: