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

Auto set tooltip for FilterBar headers? #56

Open
jhult opened this issue Aug 3, 2020 · 2 comments
Open

Auto set tooltip for FilterBar headers? #56

jhult opened this issue Aug 3, 2020 · 2 comments
Assignees

Comments

@jhult
Copy link
Contributor

jhult commented Aug 3, 2020

Should forField attempt to auto add the tooltip/title?

This is tricky because forField accepts HasValue but Component.tooltip is an extension on Component.

Here is an example of how I am currently solving it:

fun Component.tooltip(property: KProperty1<*, *>): Component = tooltip(property.name)
fun Component.tooltip(column: Grid.Column<*>): Component = tooltip(column.key)
private fun Component.tooltip(key: String): Component {
    tooltip = SharedUtil.camelCaseToHumanFriendly(key)
    return this
}

/**
 * Create a [TextField] with a [TextField.valueChangeTimeout] of 600.
 */
fun <BEAN : Any, FILTER : Filter<BEAN>> FilterBar<BEAN, FILTER>.textField(column: Grid.Column<BEAN>): FilterBar.Binding.Builder<BEAN, String, FILTER> {
    val component = TextField().apply {
        // increase filter timeout from default 400 to 600
        // 400 seems a little fast for when someone is typing in a value (in case they pause)
        valueChangeTimeout = 600

        addThemeVariants(TextFieldVariant.LUMO_SMALL)

        tooltip(column)
    }
    return forField(component, column)
}
@mvysny
Copy link
Owner

mvysny commented Aug 4, 2020

Hi, hmmm, I'm not entirely convinced this is a good idea. Just throwing ideas here, please feel free to discuss:

  1. Localization - other languages would like to use some kind of resource bundle instead of SharedUtil.camelCaseToHumanFriendly(key). However this could be solved using vok's built-in i18n; or we could read Grid.Column's header?
  2. I think the tooltip would display redundant information: the column header above would basically state the same thing than the tooltip.

What do you think?

This is tricky because forField accepts HasValue but Component.tooltip is an extension on Component.

That's actually not a problem since all Vaadin components extend from the Component class so it's safe to cast HasValue into Component.

Just an idea: What about having an utility function which creates filter text fields instead? Something like:

fun filterTextField(column: Grid.Column<*>, tooltip: String = column.header, valueChangeTimeout: Int = 600) {
  val tf = TextField()
  tf.tooltip = tooltip
  tf.valueChangeTimeout = valueChangeTimeout
}

filterBar.forField(filterTextField(this), this)...

The thing is that the timeout, theme variant and other things could be project-specific; having that code in vok would then force one project's guidelines onto another. An alternative would be to encourage programmers to develop their own set of utility functions; this encouragement could come from vok's best-practices documentation. However I wonder what the best practice would be... a global function is harder to be discovered since it won't auto-complete. So maybe having

/**
 * Create a [TextField] with a [TextField.valueChangeTimeout] of 600.
 */
fun <BEAN : Any, FILTER : Filter<BEAN>> FilterBar<BEAN, FILTER>.myProjectTextField(column: Grid.Column<BEAN>): FilterBar.Binding.Builder<BEAN, String, FILTER> {
   // ...
}

(prefix the textField function with some kind of project name prefix, or some kind of three-letter acronym coming from the company name, so that it's clear that it's project-specific function...)

@jhult
Copy link
Contributor Author

jhult commented Aug 4, 2020

  1. addColumnFor is using SharedUtil.camelCaseToHumanFriendly(key) which is why I mentioned and am using it. I think using i18n or pulling from the grid header would be fine. Perhaps addColumnFor should also use i8n.

  2. If the field label and tooltip are identical, I think you are correct that the tooltip/title is redundant information. I was trying to help accessibility but to do that (and not just clutter the UI), I probably need to provide more/different information.

Since HasValue is an interface, I'm not sure we can always cast that to a Component. Most Vaadin components may extend Component but I suppose, however unlikely, someone could create something that only implements HasValue but doesn't extend Component

Personally, I like the idea of helper functions in a library (even if I don't use them or change them). They help show me how things can (should?) be codded and hopefully give me logical defaults (such as a valueChangeTimeout of 600 ms).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants