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

Revert mdc-select to use the <select> element #2399

Closed
lynnmercier opened this issue Mar 14, 2018 · 10 comments · Fixed by #2462
Closed

Revert mdc-select to use the <select> element #2399

lynnmercier opened this issue Mar 14, 2018 · 10 comments · Fixed by #2462
Labels

Comments

@lynnmercier
Copy link
Contributor

We initially removed this because we we're trying to style mdc-select so it used mdc-menu. But we discovered that we lost a lot of native functionality with the <select> element. So we will remove mdc-select's dependency on mdc-menu, and instead use the native <select> element

@kfranqueiro
Copy link
Contributor

kfranqueiro commented Mar 14, 2018

One thing worth noting which had contributed to why we removed this: we ran into an issue due to the select no longer being the root element of the CSS-only version after it was re-styled to match Text Field Box in #1097. This caused awkwardness due to remaining top-level styles selecting on the disabled (or aria-disabled, can't remember) attribute, meanwhile the nested select was the actual element that needs to be functionally disabled. Ideally, users shouldn't need to set attributes or classes on more than the select` element itself.

If we're seeking to revert to code that existed several versions back, we should make sure to avoid this situation, presumably by making styles work with select as the root element.

Edit: I realize now that what's being referred to here is not in fact a CSS-only select, but actually updating the JS mdc-select to wrap a select element rather than rely on mdc-menu. This means that we can handle the disabled state via a setter on the component, so we can keep whatever we need to in sync.

@djensen47
Copy link

Throwing out the baby with the bath water?

@aeon0
Copy link
Contributor

aeon0 commented Apr 4, 2018

Is there a plan to make a new (better) javascript implementation? I mean, I was happy how it worked and looked in general if we'd just get rid of all the issues that came with it...

@dessant
Copy link

dessant commented Apr 4, 2018

What is the native functionality you have lost that could not have been possible to implement without reverting to the native select element?

@kfranqueiro
Copy link
Contributor

There are a few reasons we wanted to take advantage of native select:

While it's of course theoretically possible to implement these features with more JavaScript, we opted to take advantage of the platform rather than add a great deal of complexity (and bytes) to the MDC Select which would require a significant amount of time to complete and maintain.

MDC Menu will continue to exist for other use cases, but for cases where form input is involved, wrapping and reusing the native select element is the most expedient solution.

@dessant
Copy link

dessant commented Apr 4, 2018

Thanks for the reasoning, I can understand the desire to avoid the maintenance and the catch-up game with native features, though this decision will likely face contention and drive some users away.

Native select elements cannot be reliably styled, and they do look unpleasant and out of place compared to the rest of the Material Design spec, especially in desktop browsers.

@djensen47
Copy link

djensen47 commented Apr 4, 2018

... though this decision will likely face contention and drive some users away.

Native select elements cannot be reliably styled, and they do look unpleasant and out of place compared to the rest of the Material Design spec, especially in desktop browsers.

Agreed. A nice, consistent select box is on my list of must-haves for UI frameworks. If I were deciding on a UI framework now, I'd probably go to Materialize for this very reason. I could tolerate the inconsistencies of Materialize in favor of a clean, consistent and nice looking select box.

Instead, the project punted to MDC Menu, and to get a similar look it requires devs to recreate the input-looking-box (which will quickly be outdated at the pace this project is moving). So if that's the situation, why not just go ahead and create an MDC Select Menu as a separate component from MDC Select? When the time is right, combine the two but in the meantime, allow the developer to choose which one is right for their situation.

@danielweck
Copy link

I hope you guys ; or external contributors ; find the bandwidth to (re)implement the MDCMenu-based alternative to the native 'select'. As I also mentioned in this related issue ( #2552 (comment) ), I understand the rationale for favouring the native 'select' widget instead of a custom MDCMenu-based renderer. I also appreciate that the development team has limited resources and must therefore make hard choices, such as focusing its efforts on native 'select' only (i.e. complete removal of the MDCMenu alternative).

However, this breaking change from 0.33.0 to 0.34.0 (see: https://github.com/material-components/material-components-web/blob/master/CHANGELOG.md ) totally blows existing apps that make use of the old UI paradigm, which contradicts Semantic Versioning for a "minor" increment (see: https://semver.org ).

I am tempted to rollback to 0.33.0 in my project, but I would also like to keep up to date with the latest developments ... the more I postpone the inevitable upgrade, the harder it will be to update my code in the future (because of the accumulation of breaking API changes). So in the interim, I am going to (try to) put together my own MDCMenu-based "selector" component.

@nerg4l
Copy link

nerg4l commented Apr 23, 2018

The minor version (https://semver.org/#spec-item-7) you refer to in your comment states

Minor version Y (x.Y.z | x > 0) MUST be incremented if [...]

Which means only if the Major version (x) is greater than 0.

And also in the FAQ about rapid development and fast iteration (https://semver.org/#doesnt-this-discourage-rapid-development-and-fast-iteration)

Doesn't this discourage rapid development and fast iteration?
Major version zero is all about rapid development. If you’re changing the API every day you should either still be in version 0.y.z or on a separate development branch working on the next major version.

So this is not a violation of semantic versioning.

@danielweck
Copy link

@nerg4l that was my point exactly in the other issue ( #2552 (comment) )

That'll teach me for cross-posting ;)

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

Successfully merging a pull request may close this issue.

7 participants