modus-web-components icon indicating copy to clipboard operation
modus-web-components copied to clipboard

Table - Checkbox cell type

Open Kai-Richardson opened this issue 1 year ago • 6 comments

Description

In https://github.com/Trimble-Construction/cascade-web, we have a table layout and we want to render boolean values within a table. This table then gets modified by the user, and the table state gets 'applied' to the application via a button click.

Essentially, there's no good way to render boolean values currently, besides turning it into text and potentially rendering it as a badge, plaintext, or similar.

This feature does differ from the existing 'normal table view' vs 'edit table view' paradigm where the data is view-only for the table until entering edit mode. I could make the checkboxes default to disabled: false. I would be interested in the maintainers thoughts on this. The reason I didn't just add this as an edit mode was because the display was lacking as well. A possible alternative implementation would be an editable True/False badge, but I thought that was somewhat clunky and a pain for people editing values via keyboard navigation (a very common usecase).

Most of the inspiration for the code in this PR came from the implementation of Table Badge rendering in #1912. I also created a typescript type for the properties of ModusCheckbox, like was done for ModusBadge.

Future Improvements:

  • Make pressing KEYBOARD_SPACE/KEYBOARD_ENTER while focusing the table cell toggle the inner checkbox. I ran into some difficulties in implementing this properly.
  • Properly emit cellValueChanged, or perhaps not - I struggled to implement this due to the confines of editMode.

This could also be done with a Switch instead in the cell - can add those if necessary re: styleguide

Turning a feature, mode, or functionality on and off. Instead, use a Switch.

However, I think Checkboxes are correct because this does not conform the style of Switches, mainly

Toggling elicits an immediate change in the UI.

No current issue open for this, can make one if required. Relevant: #1574

IMPORTANT: I'm unsure why checkboxClick's type is changing (despite the type only being aliased), will need to fix before merging as I think that constitutes a breaking change. To me this almost reads as a bug due to our extremely out-of-date Stencil version.

Type of change

  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [x] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • [x] Documentation update

How Has This Been Tested?

I ran this by creating a table layout and passing the relevant data within the index.html set up for use with Stencil. There's currently no e2e testing for the clickability of the table component itself. Storybook works well, sort works.

Checklist

  • [x] My code follows the style guidelines of this project
  • [x] I have performed a self-review of my own code
  • [x] I have commented my code, particularly in hard-to-understand areas
  • [x] I have made corresponding changes to the documentation
  • [x] My changes generate no new warnings
  • [x] I have added tests that prove my fix is effective or that my feature works
  • [x] New and existing unit tests pass locally with my changes
  • [ ] Any dependent changes have been merged and published in downstream modules
  • [x] I have checked my code and corrected any misspellings

Kai-Richardson avatar Jun 11 '24 03:06 Kai-Richardson

Deploy Preview for moduswebcomponents ready!

Built without sensitive environment variables

Name Link
Latest commit e98bccbcf50fa55e189a4a3ffd5b1bac82156788
Latest deploy log https://app.netlify.com/sites/moduswebcomponents/deploys/6667c35958079a000844c2a8
Deploy Preview https://deploy-preview-2583--moduswebcomponents.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

Lighthouse
1 paths audited
Performance: 23 (🔴 down 1 from production)
Accessibility: 98 (no change from production)
Best Practices: 92 (no change from production)
SEO: 92 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify[bot] avatar Jun 11 '24 03:06 netlify[bot]

@coliff @egunther39 @enowak1031 For this ticket, we need to consider:

  • How do we want modus to handle this, we currently are researching custom cells
  • What styling do we want to have for view mode vs edit mode
  • Should the checkmark be centered in the cell

I could see having a boolean type column and/or a checkbox editor type. I imagine we will need the Modus Design System to weigh in on this submission as well.

@Kai-Richardson If you haven't already, can you make sure there is an Aha! submission for this so a review can be made? https://modus.ideas.aha.io/ideas

We are currently investigating custom html cells for both view and edit. If we can figure it out, that would solve for this case but it would be good to know how the Modus Design System wasn't to work with boolean values in tables.

cjwinsor avatar Jun 14 '24 22:06 cjwinsor

@Kai-Richardson Did you have an opportunity to create the Aha submission?

cjwinsor avatar Jul 03 '24 14:07 cjwinsor

@egunther39 Can you weigh in on the styling here. I don't see a "view" mode for this new checkbox column type. So the column is always an editable checkbox. Do we have an idea on how it should look when not in edit mode. Keep in mind this is not the row selection checkboxes, but a new column type for boolean fields.

cjwinsor avatar Jul 24 '24 14:07 cjwinsor

@cjwinsor - It's a bit hard to follow here. Is there a visual somewhere that illustrates the issue?

egunther39 avatar Aug 07 '24 16:08 egunther39

@egunther39 @prashanth-offcl image

The above shows the submissions checkbox column. Its only every a checkbox, with no "read only" mode such as having a modus check icon / nothing when not being edited. The UX consideration is whether we need a "read only" mode to the column as we don't currently have cells always in edit mode.

This PR introduces a column with dataType of checkbox. My suggestion is that we could support a cellEditorType of checkbox or have a dataType of boolean which when edited is a checkbox.

cjwinsor avatar Aug 07 '24 17:08 cjwinsor

This is closed, but I don't see either the dataType of boolean, nor the cellEditorType of checkbox in Modus React (^1.8.0-react18)

How is this implemented ?

axelsean avatar Jun 05 '25 15:06 axelsean