Skip to content

Fix: option with empty value#238

Merged
bendera merged 2 commits intomainfrom
fix/option-with-empty-value
Jan 3, 2025
Merged

Fix: option with empty value#238
bendera merged 2 commits intomainfrom
fix/option-with-empty-value

Conversation

@bendera
Copy link
Copy Markdown
Member

@bendera bendera commented Jan 3, 2025

Fix #237

@bendera bendera merged commit 2a64e98 into main Jan 3, 2025
@bendera bendera deleted the fix/option-with-empty-value branch January 3, 2025 12:24
@MaxKless
Copy link
Copy Markdown
Contributor

MaxKless commented Jan 3, 2025

Are we sure this is the right behaviour?

  • SingleSelect: If no option is selected, the first one should be selected by default.

I feel like it will catch some ppl off guard and might be a breaking change... With this it's not possible to have an 'unselected' selectbox anymore, right?

@bendera
Copy link
Copy Markdown
Member Author

bendera commented Jan 3, 2025

Are we sure this is the right behaviour?

I'm quite sure about it. Let's say we have this select:

<select>
  <option>lorem</option>
  <option>ipsum</option>
  <option>dolor</option>
<select>

In this case, the first option will be selected. The only way to change its state to unselected is set a non-existing value through javascript, or change its selectedIndex to -1.

I feel like it will catch some ppl off guard and might be a breaking change...

Yes, I agree. But it has been worked incorrectly until now. If I were to strictly follow semantic versioning, I would need to increase the major version now, right? Actually, I'm a bit clueless how I should handle this situation correctly.

@MaxKless
Copy link
Copy Markdown
Contributor

MaxKless commented Jan 3, 2025

Yeah with strict semantic versioning you would have to but I don't think it's important to follow it. But it might be a good idea to make it a minor instead of a patch because this is a behaviour change vs just a bug fix. Just my opinion though

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.

vscode-option with empty '' value is not supported

2 participants