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

QTabs swipeable makes text very hard to select #15503

Closed
Tofandel opened this issue Feb 28, 2023 · 8 comments
Closed

QTabs swipeable makes text very hard to select #15503

Tofandel opened this issue Feb 28, 2023 · 8 comments
Assignees
Labels
area/composables bug/1-repro-available A reproduction is available and needs to be confirmed. flavour/quasar-cli-vite kind/bug 🐞 Qv2 🔝 Quasar v2 issues

Comments

@Tofandel
Copy link
Contributor

Tofandel commented Feb 28, 2023

What happened?

When using swipeable QTabsPanel, if you try to select some text in a normal motion (not slow) instead of selecting text, it will change tab

What did you expect to happen?

The tab not to change If the user is selecting text with it's mouse

Reproduction URL

https://codepen.io/Tofandel/pen/vYzgmKN?editors=101

How to reproduce?

  1. Go to the reproduction sandbox
  2. Try to select the text in the tab left to right
  3. The selection disappears and the tab doesn't change
  4. Try to select the text right to left
  5. The tab has changed to the next tab

Flavour

Quasar CLI with Vite (@quasar/cli | @quasar/app-vite)

Areas

Composables (quasar/usePanel)
Directives (quasar/TouchSwipe)

Additional context

If you go really slow you can select the text, but it's not user friendly

If I remove swipeable then it's not mobile friendly

Proposed solution

Check the current selected text before swiping and if it's not empty, abort the swipe

function getSelectedText() {
    var text = "";
    if (typeof window.getSelection != "undefined") {
        text = window.getSelection().toString();
    } else if (typeof document.selection != "undefined" && document.selection.type == "Text") {
        text = document.selection.createRange().text;
    }
    return text;
}
@Tofandel Tofandel added kind/bug 🐞 Qv2 🔝 Quasar v2 issues labels Feb 28, 2023
@github-actions github-actions bot added area/composables bug/1-repro-available A reproduction is available and needs to be confirmed. flavour/quasar-cli-vite labels Feb 28, 2023
@rstoenescu
Copy link
Member

Hi,

The solution offered is not ok because this would hinder the functionality if any text is selected, even outside of the panel. There is no easy way out of this, except for disabling the swipe functionality.

@Tofandel
Copy link
Contributor Author

Tofandel commented Apr 26, 2023

The way to get around this would be to compare the selected text at the start of the swipe and before triggering it, if it's the same, proceed with the swipe, if it's different, abort the swipe

The swipe is extremely usefull on devices with touch enabled (and nowadays it's not just mobiles, a lot of laptops come with both touch and trackpad making feature detection buggy) so disabling it is not really an option.

Selection is not a highly needed feature by itself and can be treated low priority, but there definitely is some solutions possible

@rstoenescu
Copy link
Member

Wish it were that easy. Reopening for assessing an idea.

@rstoenescu rstoenescu reopened this Apr 26, 2023
@Tofandel
Copy link
Contributor Author

Something I noticed is this

const panelDirectives = computed(() => {
// if props.swipeable
return [ [
TouchSwipe,
onSwipe,
void 0,
{
horizontal: props.vertical !== true,
vertical: props.vertical,
mouse: true
}
] ]
})

Notice the mouse: true

Maybe we can give the option to disable mouse swipe for panel, which would also solve this issue

@rstoenescu rstoenescu self-assigned this Apr 26, 2023
@rstoenescu
Copy link
Member

Think I found a way to do it properly.
Need to implement for TouchSwipe/Pan/etc.

@rstoenescu
Copy link
Member

The gist of the fix: after swipe detection starts and mouse moves, also check if any text is currently selected (previous selection, if any, was automatically discarded when swipe has started); ALSO, we were avoiding triggering swipe for INPUT elements, but we should also do this for TEXTAREA -- and this is even more important now (besides the main obvious reason), especially for Firefox (where additional work would anyway be needed besides the getSelection() API -- see firefox bug)

Fix will be available in Quasar 2.11.11

@rstoenescu
Copy link
Member

Also, thanks for insisting with this ticket! We made Quasar better.

@Tofandel
Copy link
Contributor Author

Well, thank you for fixing this issue. All credits to you 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/composables bug/1-repro-available A reproduction is available and needs to be confirmed. flavour/quasar-cli-vite kind/bug 🐞 Qv2 🔝 Quasar v2 issues
Projects
None yet
Development

No branches or pull requests

2 participants