search

pegasystems / constellation-ui-gallery

This open-source repository provides a collection of ready-to-use and customizable Constellation DX components. Use this resource to gain inspiration, best practices, and a solid foundation for implementing custom components.
https://pegasystems.github.io/constellation-ui-gallery/
Apache License 2.0
32 stars 22 forks source link

CompareTable issues related to missing Theme tokens, HTML semantics and accessibility #8

Closed vijayjangid closed 7 months ago

vijayjangid commented 7 months ago

CompareTable component is currently implemented using HTML<table> tags and hardcoded styles for colors, spacing, borders etc, missing HTML5 table semantics causing accessiblity concerns. examples: https://github.com/pegasystems/constellation-ui-gallery/blob/75462c7fe1d7a780c6006d22cd5db961e4887de2/src/components/Pega_Extensions_CompareTableLayout/index.tsx#L173 https://github.com/pegasystems/constellation-ui-gallery/blob/75462c7fe1d7a780c6006d22cd5db961e4887de2/src/components/Pega_Extensions_CompareTableLayout/styles.ts#L76

Question: For the spreadsheet display format we could achieve the UX using OOTB Table component as well. Is there any specific reason to not use OOTB Table, any challenges in particular?

Suggestion: OOTB Table component supports column renderers and could be used to render Radio buttons within table cells. We can also easily align column data left | center | right as needed. This OOTB component does takes care of proper HTML table semantics along with accessibility.

Here are the screenshots of OOTB Table achieving similar results.

Current implementation

custom-table

Using OOTB Table

cosmos-table

OOTB Table Limitations:

  1. It doesn't support table body cells to be rendered as th. It renders every cell as td implicitely.
  2. Heading is aligned to left always. Although this can be overridden in Styles if need be.
  3. Cannot be used in complex case. i.e. displayFormat = 'financial-report'

@ricmars / @niallriddell Please suggest should we replace the implementation with OOTB Table or fix the current code to use Theme tokens, Table semantics and accessibility guidelines.

ricmars commented 7 months ago

the goal is to have a single table render that would support the complex layout of a financial report - the table should be accessible - what is the issue with the markup? th are valid per https://www.w3.org/WAI/tutorials/tables/one-header/ - the extra styles are fine - as long as the component is accessible - I am not sure there is a lot of benefits and code reduction using the OOB table component - most of the styles are for the spreadsheet use case

vijayjangid commented 7 months ago

Sure, if the goal is to use single table that's fair we can't achieve the FinancialReport using OOTB Table.

On the accessibility there were primariliy two issues since the table uses combination headers (First row and First column):

  1. Missingscope
  2. For interactive radio buttons, missing contextual information.

However, both these issues are fixed by #7

Question on the style, is this how is it designed by UX team? I can see couple of thick borders in table and semi transparent backgrounds. I was thinking if we could use Theme tokens for ootb 'table' component?

ricmars commented 7 months ago

agree - we should use the theme tokens for the borders so that it looks the same as the OOB table component and works in dark and light mode.

ricmars commented 7 months ago

closing this issue since the accessibility and semantic issues have been fixed - the use of the theme is tracked as a separate enhancement - https://github.com/pegasystems/constellation-ui-gallery/issues/20