Skip to content

Offer hiding columns from the Performance Analysis table#129

Merged
JosephMarinier merged 7 commits intodevfrom
joseph/performance-analysis-table
Jun 10, 2022
Merged

Offer hiding columns from the Performance Analysis table#129
JosephMarinier merged 7 commits intodevfrom
joseph/performance-analysis-table

Conversation

@JosephMarinier
Copy link
Copy Markdown
Contributor

@JosephMarinier JosephMarinier commented Jun 8, 2022

Resolve #126

Description:

  • Clean up
  • Offer hiding columns
    • Hide filter options (with handy boolean attribute)
    • Hide sort options since they are already available via the sorting button (in global CSS, as sx doesn't work since the popover menu is not a child of the DataGrid)
  • Have columns flex to accommodate hiding columns. Also taking this opportunity to adjust the widths to new outcome names.
  • Show columns separator in headers to avoid ambiguity with which column the menu button belongs to.
  • Refine sorting
    • Show the initial sort
    • Disable sortingOrder=null as it is redundant with sorting by utteranceCount desc
  • Set maxWidth so that columns stop getting wider going under 6 columns.

image

image

Checklist:

You should check all boxes before the PR is ready. If a box does not apply, check it to acknowledge
it.

  • PRE-COMMIT. You ran pre-commit on all commits, or else, you
    ran pre-commit run --all-files at the end.
  • FRONTEND TYPES. Regenerate the front-ent types if you played with types and routes.
    Run cd webapp && yarn types while the back-end is running.
  • USER CHANGES. The changes are added to CHANGELOG.md and the documentation, if they impact
    our users.
  • DEV CHANGES.
    • Update the documentation if this PR changes how to develop/launch on the app.
    • Update the README files and our wiki for any big design decisions, if relevant.
    • Add unit tests, docstrings, typing and comments for complex sections.

@JosephMarinier JosephMarinier self-assigned this Jun 8, 2022
@JosephMarinier JosephMarinier force-pushed the joseph/performance-analysis-table branch from 2f5819d to 6f02a43 Compare June 8, 2022 16:02
Comment thread webapp/src/styles/theme.tsx Outdated
Comment on lines 15 to 18
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it's a shame that this ends up here, but that's the best I can come up with after an hour of trying different solutions.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nandhinibsn

A general question to @JosephMarinier on the PR description. Can you explain a bit about hiding sort options? Do you mean the default sort of datagrid is disabled for all the columns in the performance analysis?

This is hiding the sort options from the column menu (visible on the first screenshot). I have not changed the options themselves.

Copy link
Copy Markdown
Collaborator

@christyler3030 christyler3030 Jun 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it really important to hide this option? Like is it bad that we can sort from two places?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not "really" important, but I thought the extra 3 buttons were making the menu overly complex, given they were not adding functionality. Also, they are not affected by the sortingOrder we define, so the null sorting option is there even if it is useless.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably fine but it seems brittle... what if MUI adds a new option to the menu?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So few letter were saved by this abbreviation. I added 5 pixels and the whole thing fits.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about Utterance Count?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with that too! I'm not sure why it was "Number of utterances" though. Maybe @christyler3030 can confirm?

@nandhinibsn
Copy link
Copy Markdown
Contributor

nandhinibsn commented Jun 8, 2022

A general question to @JosephMarinier on the PR description. Can you explain a bit about hiding sort options? Do you mean the default sort of datagrid is disabled for all the columns in the performance analysis?

- Hide filter options (with handy boolean attribute)
- Hide sort options since they are already available via the sorting button (in global CSS, as `sx` doesn't work since the popover menu is not a child of the `DataGrid`)
to accommodate hiding columns. Also taking this opportunity to adjust the widths to new outcome names.
in headers to avoid ambiguity with which column the menu button belongs to.
- Show the initial sort
- Disable `sortingOrder=null` as it is redundant with sorting by `utteranceCount` `desc`
@JosephMarinier JosephMarinier force-pushed the joseph/performance-analysis-table branch from 6f02a43 to 40237b9 Compare June 10, 2022 13:11
field: outcome,
headerName: OUTCOME_PRETTY_NAMES[outcome],
flex: 1,
minWidth: 160,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be worth having maxWidth on the columns as well for uses where there are very few columns?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ho interesting idea! 👍 Let me play with that...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, pretty cool! I just Set maxWidth so that columns stop getting wider going under 6 columns.

{
width: 206,
field: "filterValue",
headerName: OPTION_PRETTY_NAME[selectedMetricPerFilterOption],
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The headerName is not shown in the header since we have a renderHeader, but it is used in the show/hide columns menu. See second screenshot in PR description.

so that columns stop getting wider going under 6 columns.
Copy link
Copy Markdown
Collaborator

@christyler3030 christyler3030 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved with some hesitancy about the table theme override

to remove sort menu items
@JosephMarinier JosephMarinier merged commit 1145959 into dev Jun 10, 2022
@JosephMarinier JosephMarinier deleted the joseph/performance-analysis-table branch June 10, 2022 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ability to hide columns in the performance analysis table

4 participants