Skip to content
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

Minor bug fixes #38

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

colbysargentcodes
Copy link
Contributor

Line 108:
Show results if picked option is blank.
If a blank option was present, results would previously not show on focus.

Line 315:
Change setValue.val(""); to setValue("");. This will now set original
select's value to blank as intended.
Previously this would have no
effect and instead keep previously selected values.
Fixes issues #13 and #37.

Colby Sargent and others added 4 commits April 13, 2017 17:01
Line 315:
Change setValue.val(""); to setValue(""); This will now set original
select's value to blank as intended. Previously this would have no
effect and instead kept previously selected values.
Fixes issues rmm5t#13 and rmm5t#37.
Line 108:
Show results if picked option is blank.
Show results if a blank option is selected
@rmm5t
Copy link
Owner

rmm5t commented Apr 13, 2017

This appears to break the general allowMismatch usage. Testing it, it appears that it's now no longer possible to write in a custom mismatch value.

@colbysargentcodes
Copy link
Contributor Author

Apologies, forgot to copy over one more change from my own copy.

Also when retesting I noticed there's no setValue call onblur when using a mismatch, so will add this too and update soon.

@colbysargentcodes
Copy link
Contributor Author

colbysargentcodes commented Apr 13, 2017

Ok, came to realise the method I used may not be suitable for all cases (I was simply setting the value to blank and not triggering the change event if allowMismatch was true), so I've been trying to improve it a little. I've come into a couple issues... (this may be better suited in an issue thread, I'll open it if you prefer to discuss there).

  1. In my particular case, I had a blank value, so using setValue("") was fine. But in cases where there is no blank value, the original select would be set to the first option. This change event then causes the flexselect to update to the original select value.

  2. From what I can tell, allowMismatchBlank is currently useless, within the settings there is the comment:
    allowMismatchBlank: true, //If "true" a user can backspace such that the value is nothing (even if no blank value was provided in the original criteria)
    This isn't true, this also sets the value back to the first option of the original select.
    And if there is a blank value, it doesn't require any mismatch setting anyway.

Both of these can be fixed by appending a blank option on init, but the following issue still exists:

  1. due to the setValue triggering a change on the original select, the flexselect is still updated to match to either the blank value or text of an original blank value. (for example <option value=''>Select One...</option>

Proposed change

  • on init, if allowMismatch or allowMismatchBlank are true, append a blank option with an id/attribute for reference back to it.
  • add a mismatch parameter on setValue, which selects the option created above as opposed to the first blank value (due to issue 3)
  • on original select change, if the created blank option is selected, do not update val of flexselect

Unfortunately not the simplest of solutions, but it's the only way I can think of to have it working as expected in all cases.

I thought I'd get your opinion before going ahead and making these changes. Let me know what you think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants