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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix doubling attrs in NcPopover and improve docs #3876

Merged
merged 3 commits into from Mar 22, 2023

Conversation

ShGKme
Copy link
Contributor

@ShGKme ShGKme commented Mar 10, 2023

A problem

NcPopover is a wrapper around floating-vue's Dropdown component. It proxies all attrs and listeners to the wrapped Dropdown. It allows setting any Dropdown's props for customization.

But Vue inherits all attrs on the root node by default. It makes all attrs, passed as Dropdown prop, set as HTML attrs of root div.

Expected behavior with the Dropdown props

popover-expected

Actual behavior

popover-actual

The solution

This PR adds the required inheritAttrs: false.

ESLint rule helping with this problem: https://eslint.vuejs.org/rules/no-duplicate-attr-inheritance.html

Docs improvements

  • Add an example with passing props to the Dropdown
    image
  • Fix Popover positions in docs
    • Before:
      popover-wrong-position
    • After:
      popover-good-position
  • Fix typos in docs

See commits for details 馃

- Add `inheritAttrs: false` to prevent attrs doubling
- Add props passing example to the docs
Signed-off-by: Grigorii Shartsev <grigorii.shartsev@nextcloud.com>
Signed-off-by: Grigorii Shartsev <grigorii.shartsev@nextcloud.com>
Signed-off-by: Grigorii Shartsev <grigorii.shartsev@nextcloud.com>
@ShGKme ShGKme added bug Something isn't working 3. to review Waiting for reviews feature: popover Related to the popovermenu component labels Mar 10, 2023
@ShGKme ShGKme added this to the 7.8.1 milestone Mar 10, 2023
@julien-nc julien-nc modified the milestones: 7.8.1, 7.8.2 Mar 16, 2023
@mejo- mejo- merged commit 7cda57f into master Mar 22, 2023
@mejo- mejo- deleted the fix/popover-doubled-attrs branch March 22, 2023 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug Something isn't working feature: popover Related to the popovermenu component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants