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

hint and id prop are not used in NcSettingsSelectGroup #3779

Closed
raimund-schluessler opened this issue Feb 17, 2023 · 2 comments
Closed

hint and id prop are not used in NcSettingsSelectGroup #3779

raimund-schluessler opened this issue Feb 17, 2023 · 2 comments
Labels
discussion Need advices, opinions or ideas on this topic feature: settings Related to the settings component need info Not enough information provided technical debt
Milestone

Comments

@raimund-schluessler
Copy link
Contributor

The hint and id props of NcSettingsSelectGroup are not in use
https://github.com/nextcloud/nextcloud-vue/blob/3c61fd5ecd55253e01ba6716311432c95654985f/src/components/NcSettingsSelectGroup/NcSettingsSelectGroup.vue#L69-L84

This makes me wonder whether this component is actually used somewhere. A search in the Nextcloud org https://github.com/search?q=org%3Anextcloud+SettingsSelectGroup&type=code does not show any usage either. Also, it lacks a documentation example, so it's really hard to know where/how and if to use it. And, as a bonus, after the initial commit in #653 I was the only one committing to the file https://github.com/nextcloud/nextcloud-vue/commits/master/src/components/SettingsSelectGroup/SettingsSelectGroup.vue 😆 🙈

All in all, this looks like the component should be either deprecated or reworked, I guess. The reason why I ask this is that we need to migrate from NcMultiselect to NcSelect (see #3743) for the vue3 migration latest and I want to know whether it's worth the effort here.

Some input from @GretaD @juliushaertl @skjnldsv @ChristophWurst would be highly appreciated (you took care of the initial PR).

@raimund-schluessler raimund-schluessler added need info Not enough information provided discussion Need advices, opinions or ideas on this topic feature: settings Related to the settings component labels Feb 17, 2023
@raimund-schluessler
Copy link
Contributor Author

raimund-schluessler commented Feb 17, 2023

Seems it was more or less copied from https://github.com/nextcloud/richdocuments/blob/main/src/components/SettingsSelectGroup.vue which shares some of the same oddities.

@raimund-schluessler
Copy link
Contributor Author

Fixed with #4120.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Need advices, opinions or ideas on this topic feature: settings Related to the settings component need info Not enough information provided technical debt
Projects
None yet
Development

No branches or pull requests

1 participant