Fixes to remove button plugin

[Pictor13]

This replaces the pull request I made for fixing the remove_button plugin, cause of mess I made with the commits (changed the dist/ and not just the src/; and used ugly revert commit to repair).

I didn't squash the commits as each concerns a different fix/detail.

Please refer to pull request "#1151 Fix remove_button plugin when settings.maxItems=1" for details.
I am going to close that one, referring this pull request.

PS - Sorry for taking so much time to recreate it. Hope it will help somebody.

[coveralls]

Changes Unknown when pulling ea2e931 on Pictor13:fix-remove_button_plugin into ** on selectize:master**.

[anx-ckreuzberger]

I just tried your "remove_button" and the "x" icon just magically appeared, without having to change any of my configuration. Thank you so much!

[temuri416]

Not sure this is working as intended for the case when settings.maxItems=1.

If you edit

https://github.com/selectize/selectize.js/blob/master/examples/plugins.html#L38

and add maxItems: 1 to the config, your selectize control will look like this:

[Pictor13]

Update

Had no time to check all the little details and the consequences; sorry if this PR stalled.
Atm I am trying to:

• check the bugs or little disfunctions coming with this PR
• check what was the original logic intended by the author (regarding the combination of the implicitly dependant settings: maxItems, mode, allowEmptyOption)
• check what the ChoppyThing's PR broke of that logic and original feature (IMO it broke in different points)
• confirm all the use-cases are ok before confirming this PR
• (eventually) add unit-test or add some documentation about the insights that I get to

Bugs

• in a project of mine the PR fix seems to provoke another bug:
  selecting and then removing an item, got from a query A (with load), won't be shown in the dropdown, when trying to reload (typing) the same query A; running a query that was not previously cached, and then trying again the previous steps, seems to restore the missing result in the dropdown.
  Not sure it is related, but maybe the plugin rendering steps on the rendering called by load.
• with maxItems: 1 or mode: single, providing two or more values in initialisation (as value of the original HTML element) seems to load them (see plugin-example page), rather than skipping the too many items.

ToDo

• 🔳 verify if it happens always and find a fix.
• ☑️ check bug reported by @temuri416 and find a fix
• • 🔳 verify why remove-button seems to not appear, in single-mode, with 0.12.3 while it does with the 0.12.6
• 🔳 verify everything works in multi/single mode and in multi mode with maxItems=1
• 🔳 verify the plugin setting append: false
• 🔳 verify the points reported in ChoppyThing's pull-request comments
• 🔳 (if possible/enough time) add unit-tests

To understand

(in order to stay consistent and don't have breaking/unintended consequences)

• is single mode an implicit minItems: 1? (if that setting would exist)
  I think so, according to what a single HTML <select> is: at least one option must be selected. No; selectize always support the removal of items
• removeItem removes the item without checking if it is single or multi mode or if allowEmptyOption is enabled; makes me think that it doesn't matter if a single-select becomes empty (contrary to the previous point). <-- indeed
  Todo: verify behaviour when triggering removeItem on a single-select with allowEmptyOption: false; then try it with allowEmptyOption: true.
  Todo: in the first case, verify whether the congruency-check for the number of items, in single-mode, is done somewhere else (during rendering/refreshing?) or if it is done at all. (misunderstood)
• is mode setting safe enough? In Make Single Select Look like Tags #547 is mentioned to not be documented; are there reasons for that? Is it broken/unsafe in some circumstances? mode looks safe to me
• shouldn't a control with maxItems: 1 avoid at all to load more than one item? Atm, as you can see in the plugin-examples page, it won't be able to select more than one value, but it is initialised with the two values provided (neat, awesome); shouldn't the rendering/refresh methods ensure to not render more items than specified in maxItems? Why 2 items are rendered (it should just have 2 options and 1 item: the first or the last)
  This is a bug, but not of concern with this pull-request

[Pictor13]

@temuri416 that's how it is supposed to look like.

Why

It changes visually because, when you just specify maxItems: 1, Selectize falls back to single mode so like a normal single <select>.
(also the CSS will be for single, so no styling for items as cards/boxes, since there is just a single choice possible).
If you remove the render setting, that wraps the item-value with "-quotes, it will look like as expected (single select).

Note: the position of the "x" is maybe a little misaligned, but it is not a bug. Maybe the less file should be adjusted.

Solution

If you wanna get the cards/boxes styling you need to manually force the multi mode via the setting mode: 'multi' when initialising.

(I didn't notice because in my project I tend to always specify the desired mode rather than making it implicit by combination of settings)

By the way....

• I am not sure forcing the mode is completely safe though: @brianreavis didn't add the setting to the documentation so I am not sure if this is supported and working in the 100% of cases.
  probably a useless concern; using mode looks fine to me
• Also to notice that Brian's code, initially, was preventing to have the remove-button in single mode.
  And it makes sense to me, since in that mode the behaviour should be the same as a normal html-single-<select> (at least one option must be selected). But this has been changed arbitrarily by @ChoppyThing in her pull-request, that I am trying to fix
  Wrong, it is still possible to remove the choice using [backspace]
• doesn't matter; I misunderstood what that setting does.
  the remove_button plugin was developed before the implementation of the allowEmptyOption setting and (I suppose) it was not taken in account at that time.
  IMO the single mode should support the remove_button plugin just when allowEmptyOption is enabled. Does it make sense to others than me?

[Pictor13]

I fixed the styling. Now looks decent also with single-selects.

Is there something more to fix here, or can we merge?

[Pictor13]

When merged this it could close also #124, #123 and #113.

[Pictor13]

Referencing the pull-request #1303 because of some insights in its comments, that are useful in evaluating also he changes made in here.

[github-actions]

Stale pull request message