insightsengineering / teal.slice

Reproducible slice module for teal applications
https://insightsengineering.github.io/teal.slice/
Other
11 stars 5 forks source link

Filter card summary design #220

Closed asbates closed 1 year ago

asbates commented 1 year ago

Review and improve the design of the filter card summary.

Some comments from @nikolas-burkoff

Arrow would be better for displaying selected range of numeric columns.

image

If there aren't any NA/Inf maybe we shouldn't show NA/Inf

image

lcd2yyz commented 1 year ago

Agree regarding NA/Inf. Also can we reduce the spacing between the two and right-align a bit more? To leave more space for the actual selected values

Hesitant about using arrow on the selected range though. We simply don't tend to see that type of notation typically. I think maybe instead of the short dash - (which is the same as the negative sign), perhaps using a longer em dash — is better? eg. -2.7 - -1.85 vs -2.7 — -1.85

lcd2yyz commented 1 year ago

Also would like to add some conditional handling in the summary for categorical variables (checkbox groups UI)

image

asbates commented 1 year ago

For NA/Inf, I don't think displaying just "NA" or "Inf" conveys much information. What I mean is I don't think "NA" conveys to the user that missing values are kept. We should rethink not just the alignment here but how the message is conveyed.

lcd2yyz commented 1 year ago

Yes agree, I thought it was something still WIP, so I didn't comment. So can we go back to this? Originally posted to #181 image

We can right align the text to show Includes: NA, Inf with a check or cross mark behind the NA/Inf. If there are no NA/Inf values in the data, then don't show in "Includes"

donyunardi commented 1 year ago

Levels selected

Also would like to add some conditional handling in the summary for categorical variables (checkbox groups UI)

  • If all available levels are selected (basically no filtering applied), then display "All levels selected".
  • If only 1-2 levels are selected (for variable with more than 2 levels available), display the full selected value (eg. M, F for variable SEX)
  • If >=3 or more levels selected, then keep display as-is

I think we talked about this issue before, and it has to do with variables that have really long values. For example, the RACE variable has options like "American Indian or Alaska Native" and "Native Hawaiian or Other Pacific Islander." If someone picks both of these options, it might be hard to fit them both on one line when we show them on a card.

One idea we had was to cut off the variable at a certain length. But we realized that this might not be the best solution if the first option is really long, because then the second option might not show up.

NA / Inf

For NA/Inf, I don't think displaying just "NA" or "Inf" conveys much information. What I mean is I don't think "NA" conveys to the user that missing values are kept. We should rethink not just the alignment here but how the message is conveyed.

Yes agree, I thought it was something still WIP, so I didn't comment. So can we go back to this? Originally posted to #181 We can right align the text to show Includes: NA, Inf with a check or cross mark behind the NA/Inf. If there are no NA/Inf values in the data, then don't show in "Includes"

Couple thoughts on this:

  1. I'm trying to determine whether adding the word "Include" would enhance clarity. At present, we're employing the term "Keep" in the selection. Should we keep it the same way?

  2. Currently, we display both the "from" and "to" values for dates. However, having an "NA" on the right side does not appear visually appealing to me.

  3. Should we consider providing a sentence instead? "Data includes NA and infinite values"

  4. Should the information about NA and Inf added on the new line after "x levels selected" label?

  5. Should we use tooltip to show the rest of information between user hover over "x levels selected" label?

donyunardi commented 1 year ago

Acceptance Criteria

This is a research ticket and we will execute on different issue.

lcd2yyz commented 1 year ago

I will try to draft up some more harmonized and standardized filter card designs. Please help to identify all the possible combinations of filter class/type/state that we may have. Following are the list of the top of my head.

By filter class

By filter variable type

By filter card state

Any more I missed?

donyunardi commented 1 year ago

I think you covered most of them. The only one that I think of is logical variable type.

chlebowa commented 1 year ago

By filter class

  • [ ] Interactive (with default set by API)
  • [ ] Interactive (user created in-app)
  • [ ] Fixed
  • [ ] Grouped?

This no longer reflects our design. There are no separate classes for interactive/fixed filter states and no separate classes for filters created with API on start-up or by the user during runtime.

There is FilterState class (with subclasses for different data types). A FilterState object defines subsetting on one variable only. A FilterState can be created on start-up, by the API (as defined by the app developer) or during runtime, by the user. A FilterState can be disabled and re-enabled by the user. A disabled FilterState cannot be modified. A FilterState created by the API can be created disabled. This can be changed by the user. A FilterState created by the API can be created fixed, which means none of its properties can be modified. It can still be disabled.

By filter variable type

  • [ ] Numeric
  • [ ] Dates/datetime
  • [ ] Logical
  • [ ] Factors
  • [ ] Characters (eg. more than 5 factors?)

All variable types except logical create a ChoicesFilterState if there are less than N unique values, N being defined by a teal option nad being constant throughout the session. ChoicesFilterStates always converts data to a factor allows choosing values with checkboxes. When >N unique values are present in the data, selection defines ranges, whether the data is numeric, date, or datetime. ( If a factor is passed that has >N unique values, a drop-down is used instead.)

By filter card state

  • [ ] Card expanded
  • [ ] Card collapsed

Any more I missed?

lcd2yyz commented 1 year ago

Finalized the filter card summary design and shared general thoughts with @donyunardi

First, for all interactive filters - by variable type

image image

For fixed filters, with single variable in condition

image

For grouped filters

image

Lastly for a case that generated some debates - fixed filters with multi-variable conditions

image image image

Sorry for all the screenshots, I did this in PPT and not able to share an internal doc here. I will send gSlide link via group chat.

Design updated on Apr 21, 2023 Changes from previous version, based on Apr 20 discussion

chlebowa commented 1 year ago

Currently the summary of a disabled filter card spells "Disabled". Are we to depart from that? Please confirm.

lcd2yyz commented 1 year ago

Currently the summary of a disabled filter card spells "Disabled". Are we to depart from that? Please confirm.

Thank you very much for pointing it out! Let's go with active/inactive when we talk about state of the filter cards. Modified in the mockup above as well.

BLAZEWIM commented 1 year ago

IMO while Potential solution 2 looks nicer, it will get messy when more complex filter are applied.

Imagine group filters to be nested, so the parts of group filers might be grouped filters, like: (SEX = M AND AGE > 45) OR ((SEX = K OR (SEX = M AND AGE < 15)) AND (SCORE > 15)) It will look terrible on solution 2.

Potential solution 1 is more robust, and you can fit everything you want, plus we expect users and "receivers" to know what &, | and brackets mean.

asbates commented 1 year ago

I'd like to make a few comments on the use of color here.

  1. I think this is generally going to be too much color and will be distracting for a user. Generally it's a good idea to use color sparingly, usually to make something stand out to focus the users attention.
  2. If we really want to use a lot of color, we should use the colors for their intended roles. With Bootstrap there a defined colors called primary, secondary, warning, error, info, a whole host of grays, etc. Using the primary color for everything doesn't display distinction between content. For example, I would consider the conditions (e.g. AESER == 'Y') information so would use the "info" color here if any (non-gray) color at all.
  3. Consider the card header. The majority has a lighter blue with the switch being a darker blue. That is going to be difficult to maintain across themes. For a dark theme for example, would the switch be a lighter shade? What if someone uses a fully custom theme? (i.e. not an off-the-shelf Bootswatch theme). While it may look fine if we only use the default theme, it's going to be very difficult to maintain across any color scheme. And if we keep it blue, it will clash if a user chooses a different theme. Which brings me to...
  4. I think for now we should focus on content. Things like alignment are fine because that's going to be common whatever the theme. But things like color and/or something that won't apply no matter the underlying theme should wait. The reason I say this is because the entire teal ecosystem needs a visual overhaul and making changes to one thing at a time without taking everything into consideration will lead to visual clashes. One example of this is sometimes when you see the default Shiny blue color, it is hard coded rather than actually matching a theme (examples below). If you view an app with the variable browser module you'll notice the colors of the plots don't match the theme.

Example of clash that results from hard-coded colors that don't respect the theme. This Bootstrap version 5 with Bootswatch theme of "Journal". Note the color for the radio buttons. The buttons themselves are the primary theme color. The label background however is based on the default Shiny colors. The result is a pretty stark visual clash.

Screenshot 2023-04-21 at 08-47-37 Screenshot

lcd2yyz commented 1 year ago

4. I think for now we should focus on content.

Agree focus should be on content and content arrangement as priority. Coloring per bootstrap theming is nice to have at this point, perhaps can be tackled together with the theming of the UI component (eg. progress bar/distribution) later. Will open a separate issue.

asbates commented 1 year ago

I can't remember if this was discussed but I think displaying "NA" in the selected value section is redundant.

image

I'd recommend sticking with just the one 'NA' on the right side, like so.

image

@lcd2yyz is that OK with you?

lcd2yyz commented 1 year ago

@asbates Yes agree - should display NA right-aligned only. Thanks!

lcd2yyz commented 1 year ago

@asbates Sorry one more minor detail... if the NA is an explicit data point as a character string "NA", not the missing data point NA or NA_character, then it should still show up in the selected value section (in addition to the right-aligned NA-checkmark if there is missing data). Hope that make sense?

chlebowa commented 1 year ago

That would be the intended behavior for a character/factor.

asbates commented 1 year ago

@lcd2yyz can you provide some examples of input and output regarding numeric values and scientific notation?

lcd2yyz commented 1 year ago

@lcd2yyz can you provide some examples of input and output regarding numeric values and scientific notation?

Added an example in this issue https://github.com/insightsengineering/teal.slice/issues/251. We can continue discussion about numeric value and scientific notation there?