quasarframework / quasar

Quasar Framework - Build high-performance VueJS user interfaces in record time
https://quasar.dev
MIT License
25.99k stars 3.53k forks source link

Update default clickable behavior for q-item #16731

Open Flamenco opened 11 months ago

Flamenco commented 11 months ago

What happened?

I need to add a clickable property for every v-item that I create even though they have a click handler.

What did you expect to happen?

v-item should be clickable if:

1) a click handler and NO clickable property 2) clickable property present and set to truthy value

const isClickable = computed(() => {
  if (props.clickable === undefined) {
    return typeof props.onClick === 'function'
  } else {
    return !!props.clickable
  }
})

Although this may apply to other Quasar components, I am focusing on v-item because my app has thousands of them in menus, and the current behaviour has pushed me to my limit :)

Reproduction URL

https://codepen.io/Flamenco/pen/XWGmWYo

How to reproduce?

Click on items with click handler and no clickable property.

Flavour

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

Areas

Components (quasar)

Platforms/Browsers

No response

Quasar info output

No response

Relevant log output

No response

Additional context

As a hint on how to implement this in Vue3, here is how I solved this issue with my personal override of q-item.

q-simple-item is a component that allows me to create items like this:

<q-simple-item icon='help' label='Show Help' @click="onShowHelp" />

Notice how I need to declare onClick in the props to detect it. This is because of this issue: https://github.com/vuejs/rfcs/discussions/397

Also notice that clickable must NOT be declared as "optional boolean" to detect it.

Here is the implementation.

<script setup lang="ts">
import {computed} from 'vue'

const props = defineProps<{
  label: string
  icon: string
  clickable?: any
  onClick?: Function
}>()

const isClickable = computed(() => {
  if (props.clickable === undefined) {
    return typeof props.onClick === 'function'
  } else {
    return !!props.clickable
  }
})

const emit = defineEmits(['click'])
</script>

<template>
    <q-item v-bind="$attrs" :clickable="isClickable" @click="emit('click')">
      <q-item-section side>
        <q-icon :name="icon"/>
      </q-item-section>

      <q-item-section>
        <q-item-label>
          {{ label }}
        </q-item-label>
      </q-item-section>
    </q-item>
</template>
yusufkandemir commented 10 months ago

Some other components have similar behavior to what you want. For example, QTable detects the presence of @row-click and changes the cursor, adds hover effects, etc. I don't know why QItem has been designed this way, there might be a good reason behind it. /cc @rstoenescu @pdanpdan

But, know that we can't make this change for v2 as it would be a breaking change. But, there is no problem making this change, we can consider it for Quasar v3.

Flamenco commented 10 months ago

With all due respect, "can't" is a mighty big word.

I was thinking something like this.

app.use(Quasar, {
   experimental: {
      enable_qitem_when_handled: true
  }
})

And then towards v3, if your users overwhelmingly agree this is the proper behaviour, you can make it default, and a breaking change.

Flamenco commented 10 months ago

Also, the only way I see this is breaking is if a user has a handler and NO attribute. And that would mean the item would not be clickable, with a handler that would never be called; That combination does not really make much sense.

yusufkandemir commented 10 months ago

@Flamenco while that is a way that makes it possible, our time or processes are not really suitable to create/maintain these experiments. One could also argue whether this small behavior is worth the trouble and something desperately needed by the users, especially considering it has been like this for about 6 years: image.

Also, the only way I see this is breaking is if a user has a handler and NO attribute

Someone might have a handler but might have something like :clickable="someCondition" to toggle the clickable behavior. In this case, if we solely look for the presence of a handler, it would break their logic and make the item always clickable.

Flamenco commented 10 months ago

When we use this approach with an arguably better default, we do not call it experimenting, we call it staging.

Furthermore, if someone has the use case you indicated where "Someone might have a handler but might have something like :clickable="someCondition", that is NOT APPLICABLE to my suggestion and will not break. This implementation is ONLY WHEN A HANDLER IS PRESENT WITH NO CLICKABLE ATTRIBUTE PRESENT.

I would wager that 99.99% of your users have a handler and are then forced to add the 'clickable' attribute with the default 'true' value, and .000001% have a clickable attribute with no handler at all. Methinks this is enough to warrant the implementation.

IMO it has been six years of not fixing a simple default. Anyway, at this point I'm glad to have felt out the water, and not have spent the effort with a PR.