insightsengineering / teal.modules.clinical

Provides teal modules for the standard clinical trials outputs
https://insightsengineering.github.io/teal.modules.clinical/
Other
32 stars 17 forks source link

[Feature Request]: move selected icon in `Select patient` in tm_t_pp_basic_info to the left side #802

Closed m7pr closed 1 year ago

m7pr commented 1 year ago

Feature description

While working on a sample module

library(teal.modules.clinical)
adsl <- tmc_ex_adsl

app <- init(
  data = cdisc_data(
    cdisc_dataset("ADSL", adsl)
  ),
  modules = modules(
    tm_t_pp_basic_info(
      label = "Basic Info",
      dataname = "ADSL",
      patient_col = "USUBJID",
      vars = choices_selected(
        choices = variable_choices(adsl),
        selected = c("ARM", "AGE", "SEX", "COUNTRY", "RACE", "EOSSTT")
      )
    )
  )
)
if (interactive()) {
  shinyApp(app$ui, app$server)
}

you can notice that the selection icon for Select patient overlaps the patient barcode. This is because patient code is too long. What if we move the icon to the left side of barcodes? So that we have icons first, and then patient barcodes

image

Code of Conduct

Contribution Guidelines

Security Policy

chlebowa commented 1 year ago

Is that even possible? :thinking: ?shinyWidgets::pickerOptions

m7pr commented 1 year ago

Check this @chlebowa https://stackoverflow.com/questions/41571413/css-bootstrap-select-move-show-tick-mark-to-the-left-of-text-in-dropdown-menu

pickerOptions uses https://developer.snapappointments.com/bootstrap-select/options

m7pr commented 1 year ago

maybe we can consider that in all pickerOptions that has picker-labels longer than SOMETHING

lcd2yyz commented 1 year ago

Seems like some CSS problem when screen size is narrowed? The big white space seems to be placeholder for the checkmarks? image

m7pr commented 1 year ago

I wonder, if the CSS change suggestions from above StackOverflow questions helps with the issue for narrowed screen size.

    .bootstrap-select.btn-group.show-tick .dropdown-menu li.selected a span.check-mark{
    position: absolute;
    display: inline-block;
    left: 5px; //changed from right:15
    margin-top: 5px;
}

Will try this out

averissimo commented 1 year ago

Given the order of the elements, wouldn't a change of position to relative work? The downside would be an empty space on the left on non-selected elements

Crude PoC below the screenshot

image

library(teal.modules.clinical)
adsl <- tmc_ex_adsl

style = "
#teal-main_ui_container .bootstrap-select.show-tick .dropdown-menu .selected span.check-mark {
  position: relative;
  visibility: visible;
  top: 0;
  right: 5px;
}

#teal-main_ui_container .bootstrap-select .dropdown-menu li a span.check-mark {
  visibility: hidden;
  display: inline-block;
}
"

app <- init(
  data = cdisc_data(
    cdisc_dataset("ADSL", adsl)
  ),
  header = shiny::div(shiny::tags$h1("I am a title"), shiny::tags$style(style)),
  modules = modules(
    tm_t_pp_basic_info(
      label = "Basic Info",
      dataname = "ADSL",
      patient_col = "USUBJID",
      vars = choices_selected(
        choices = variable_choices(adsl),
        selected = c("ARM", "AGE", "SEX", "COUNTRY", "RACE", "EOSSTT")
      )
    )
  )
)
if (interactive()) {
  shinyApp(app$ui, app$server)
}
lcd2yyz commented 1 year ago

@averissimo Nice work! I actually do like the checks showing on the left side better than the original! The blank space will be indicative that it's not selected. Can we get a few more opinion on this before committing?

m7pr commented 1 year ago

@averissimo looks, great. I'm a fan of the solution

averissimo commented 1 year ago

I'm glad I could help! :relaxed: This one has my vote, although I think the sizes can be adjusted to squeeze in a bit tighter

I can take care of the implementation @m7pr if you wish to pass this one over

m7pr commented 1 year ago

Go ahead @averissimo - I never intended to fix. I just raised the ticket for a potential feature request

chlebowa commented 1 year ago

It's perfectly fine with the empty spaces. I agree that the tick could be a tad smaller but It don't think it's necessary.

lcd2yyz commented 1 year ago

Just want to be clear about the scope of this change - this is to be implemented at teal.widget level, for all UI using select picker input then?

averissimo commented 1 year ago

@lcd2yyz Applying this to all select picker input would deliver a consistent UX across teal.

Can you think of any input that should NOT have this behaviour?

lcd2yyz commented 1 year ago

@averissimo Nothing comes to mind. I agree that change should be applied to all select picker for consistency.