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

fix(NcSettingsSelectGroup): Fix "this.getValueObject is not a function" #4104

Closed
wants to merge 4 commits into from

Conversation

nickvergessen
Copy link
Contributor

Tried to use the component in an app, but it fails to load.

  • So I wrote a docs sample and it had the same issue.
  • Fixed the bug
  • Remove unused property
  • Try to extend the example (but I need to fake the response of the following request, do we have a sample how to achieve that?
    • GET /ocs/v2.php/cloud/groups/details?search=&limit=10
  • Migrate the component from deprecated NcMultiselect to NcSelect

Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen nickvergessen added bug Something isn't working 2. developing Work in progress feature: select Related to the NcSelect* components labels May 12, 2023
@nickvergessen nickvergessen added this to the 8.0.0 milestone May 12, 2023
@nickvergessen nickvergessen self-assigned this May 12, 2023
@raimund-schluessler
Copy link
Contributor

raimund-schluessler commented May 12, 2023

The id prop is also not used, and the component seems to be in a bad shape generally. I guess it was never really used by an app. See #3779 as well.

@raimund-schluessler
Copy link
Contributor

  • Try to extend the example (but I need to fake the response of the following request, do we have a sample how to achieve that?

    • GET /ocs/v2.php/cloud/groups/details?search=&limit=10

This function from NcSelectTags might help. In the vue docs, some fake tags are returned there:

https://github.com/nextcloud/nextcloud-vue/blob/55b2b9f4611a59815a6be1aca7d97fa45c07b9f1/src/components/NcSelectTags/api.js#L91-L111

@nickvergessen
Copy link
Contributor Author

I guess it was never really used by an app.

Yeah, couldn't find any usage and well at the moment it wouldn't even load....

@nickvergessen
Copy link
Contributor Author

The id prop is also not used, and the component seems to be in a bad shape generally.

Thanks, will see if we use the id instead or drop it

This function from NcSelectTags might help. In the vue docs, some fake tags are returned there:

Thanks a lot 🙇🏼‍♂️

@ShGKme
Copy link
Contributor

ShGKme commented May 12, 2023

This function from NcSelectTags might help. In the vue docs, some fake tags are returned there:

If I understand correctly, how it works, it adds documentation-related runtime to the component.

We can also mock requests in the documentation without modifying the final component:

  1. By mocking @nextcloud/axios using webpackConfig.resolve in the styleguidist
  2. By mocking HTTP in the styleguidist with mswjs.io so the component may work in the documentation most closely to the real world
  3. Or by using webpack.definePlugin instead of if (window.NextcloudVueDocs). Then component will be modified for documentation, but it won't be a part of the bundle. (Same as process.env.NODE_ENV is often used in js libs)

@nickvergessen nickvergessen marked this pull request as draft May 12, 2023 15:25
Signed-off-by: Joas Schilling <coding@schilljs.com>
@raimund-schluessler raimund-schluessler deleted the bugfix/noid/fix-NcSettingsSelectGroup branch August 25, 2023 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Work in progress bug Something isn't working feature: select Related to the NcSelect* components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants