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

Consistency in reporting component UI #636

Closed
nikolas-burkoff opened this issue May 20, 2022 · 3 comments
Closed

Consistency in reporting component UI #636

nikolas-burkoff opened this issue May 20, 2022 · 3 comments
Labels
core enhancement New feature or request

Comments

@nikolas-burkoff
Copy link
Contributor

nikolas-burkoff commented May 20, 2022

In example app here: #635 (comment) we have specified the reporting components directly:

encoding = tagList(
          fluidRow(
            column(4, teal.reporter::add_card_button_ui(ns("addReportCard"))),
            column(4, teal.reporter::download_report_button_ui(ns("downloadButton"))),
            column(4, teal.reporter::reset_report_button_ui(ns("resetButton")))
          )

Which can cause problems:
image

We might be able to use simple_reporter instead but does this also cause problems? We should be consistent in how and where module developers ought to specify the reporting ui

and this should be explained in #632

@nikolas-burkoff nikolas-burkoff added the enhancement New feature or request label May 20, 2022
@nikolas-burkoff nikolas-burkoff changed the title Standard reporting component Consistency in reporting component UI May 20, 2022
@kpagacz
Copy link
Contributor

kpagacz commented May 20, 2022

I firmly believe this should be solved with CSS. The buttons should be responsive on their own.

@kpagacz
Copy link
Contributor

kpagacz commented May 20, 2022

Looking at the CSS classes we have for these UI elements, seems we piggyback off of bootstrap which is recommended. The fluidRow and columns set up fixed positions for the buttons that override a sensible flex behaviour. In short - don't use fluidRow and columns because they are junky.

@kpagacz kpagacz closed this as completed May 20, 2022
@Polkas
Copy link
Contributor

Polkas commented May 20, 2022

I just update the example app in the main PR to use the regular div.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants