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

tm_missing_data reporting issues #471

Closed
Tracked by #44
nikolas-burkoff opened this issue Oct 4, 2022 · 8 comments
Closed
Tracked by #44

tm_missing_data reporting issues #471

nikolas-burkoff opened this issue Oct 4, 2022 · 8 comments
Labels

Comments

@nikolas-burkoff
Copy link
Contributor

nikolas-burkoff commented Oct 4, 2022

From #469

  • The report card doesn't actually say what dataset is being considered - maybe this extra line could be added in

  • Is this what we want? Especially because when we reorder the table on-screen - it does not take the reorder into account when adding the tibble to the report card (should it?)

image

@nikolas-burkoff nikolas-burkoff changed the title tm_missing_data "By variables level table" in report card tm_missing_data reporting issues Oct 4, 2022
@donyunardi
Copy link
Contributor

donyunardi commented Oct 5, 2022

The report card doesn't actually say what dataset is being considered - maybe this extra line could be added in

I agree. The card should have the dataset name since we're dealing with multiple dataset.
@kumamiao do you agree with this?

Is this what we want? Especially because when we reorder the table on-screen - it does not take the reorder into account when adding the tibble to the report card (should it?)

Good finding about reordering not being retain during the card addition. Since this is for missing data, I don't think the reorder matters as long as it shows the missing results. It would be great though if the card can also retain the reorder happened on-screen.
@kumamiao what do you think?

I also think these issues should be in backlog.

@mhallal1
Copy link
Collaborator

mhallal1 commented Oct 5, 2022

The first point is a one liner fix, the second point is probably because the table is a DT table, adopt table_with_settings maybe?

@kumamiao
Copy link

kumamiao commented Oct 5, 2022

If the first point is a one liner fix, would be great to go ahead and address it in the UAT if feasible. I'm okay with putting the second into the backlog for next release if it's not that straightforward to fix.

@donyunardi
Copy link
Contributor

For the first issue, I'm fine with pushing the fix during UAT if the risk is minimal.

Are we thinking about updating the card name and the content, or just the content?
@kumamiao can you give an input on how we should mention the dataset name? Would it be the same line with the title or should it be a new line, etc?

@mhallal1
Copy link
Collaborator

mhallal1 commented Oct 6, 2022

First issue is addressed here: #473

@nikolas-burkoff
Copy link
Contributor Author

Moving second item to backlog

@chlebowa
Copy link
Contributor

chlebowa commented Aug 3, 2023

Acceptance criteria:
Ensure the table in the report reflects the state of the table in the app, including user-directed sorting.
Make sure to include sorting by missing values.

@donyunardi
Copy link
Contributor

As discussed with @kumamiao, we don't need to have the report reflects the user-directed sorting.
Closing.

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

No branches or pull requests

5 participants