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: Adding an extra props "unSelectable" to QTree (fix #7701) #12012

Merged
merged 2 commits into from
Jan 15, 2022

Conversation

super-seo
Copy link
Contributor

@super-seo super-seo commented Jan 12, 2022

Hi.
Example
1.https://codepen.io/superseo/pen/XWeyBpz
2.https://codepen.io/superseo/pen/oNGQMQO?editors=101

Taken from the manual https://quasar.dev/vue-components/tree

The second click on the selected element always resets the selection. That is, click on the element for the first time - the corresponding Tab Panel opens, click again on the same element - it closes.
But it is necessary that the second click on the same element does not cause deselection.

Adding an extra props "unSelectable" is probably enough. By default (true) - old behavior. If false, then the second click does not deselect. For those cases when such behavior is necessary, without breaking the old logic.

https://codepen.io/superseo/pen/xxXQJLz?editors=101

added a new parameter, for example on PR fix: Adding an extra props "unSelectable" to QTree (fix #7701) #12012
just use <q-tree ... :un-selectable="false" />
The error from the description occurs because of a check on this line
https://github.com/pdanpdan/quasar/blob/8a5a2d799bc4af2dc185ca0ec1614b7dd66737aa/ui/src/components/tree/QTree.js#L607

Same request but for v1
#3551

What kind of change does this PR introduce?

  • Bugfix
  • [*] Feature
  • Documentation
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change?

  • Yes
  • [*] No

The PR fulfills these requirements:

  • [*] It's submitted to the dev branch (or v[X] branch)
  • [*] When resolving a specific issue, it's referenced in the PR's title (e.g. fix: #xxx[,#xxx], where "xxx" is the issue number)
  • It's been tested on a Cordova (iOS, Android) app
  • It's been tested on a Electron app
  • Any necessary documentation has been added or updated in the docs or explained in the PR's description.

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature (to avoid wasting your time, it's best to start a new feature discussion first and wait for approval before working on it)

Other information:

@rstoenescu
Copy link
Member

Hi,

Thanks for contributing!
Changed the implementation though:

  1. Boolean props should retain their default "false" value because this allows for easier usage in templates. Just the presence of that prop should be enough (no-selection-unset vs :no-selection-unset="true/false").
  2. Improved the emit logic.

@sbscan
Copy link

sbscan commented Jan 16, 2022

Hi,

Thanks for contributing! Changed the implementation though:

  1. Boolean props should retain their default "false" value because this allows for easier usage in templates. Just the presence of that prop should be enough (no-selection-unset vs :no-selection-unset="true/false").
  2. Improved the emit logic.

When we use selectable nodes, we can only open the node by clicking the arrow. I think UX will be better if it's flagged selectable:false clicking it should open the node. Specially on mobile trying to click the arrow is not a good practice.

@super-seo
Copy link
Contributor Author

super-seo commented Jan 17, 2022

Hi,
Thanks for contributing! Changed the implementation though:

  1. Boolean props should retain their default "false" value because this allows for easier usage in templates. Just the presence of that prop should be enough (no-selection-unset vs :no-selection-unset="true/false").
  2. Improved the emit logic.

When we use selectable nodes, we can only open the node by clicking the arrow. I think UX will be better if it's flagged selectable:false clicking it should open the node. Specially on mobile trying to click the arrow is not a good practice.

Hi.
This edition is not about list disclosure. This is a refinement about the ability not to remove the selection on the second repeated click. You should probably output a separate task/request.

However, in your case with selectable=true you can watch the emit 'update:selected' and use the tree's setExpanded(selected, true) method.

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.

When second click to selected node on QTree selected key becomes null
3 participants