Skip to content

Table: Set fixed check box width , new prop for setting row action column width #2874

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ElishaSamPeterPrabhu
Copy link
Collaborator

@ElishaSamPeterPrabhu ElishaSamPeterPrabhu commented Sep 19, 2024

Description

  • Added column width property for row action column
  • Set a fixed width to check box

References
Fixes #1922

Type of change

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

How Has This Been Tested?

Checklist

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

Copy link

netlify bot commented Sep 19, 2024

Deploy Preview for moduswebcomponents canceled.

Name Link
🔨 Latest commit 9908d8e
🔍 Latest deploy log https://app.netlify.com/sites/moduswebcomponents/deploys/66fa74f9f5314f000871e617

@coliff
Copy link
Member

coliff commented Sep 19, 2024

Hello! Can you please provide steps to test/review the changes? I tried changing the rowActionSize at: https://deploy-preview-2874--moduswebcomponents.netlify.app/?path=/story/components-table--checkbox-row-selection&args=rowActionSize:99999 but it didn't appear to have any effect.

@ElishaSamPeterPrabhu
Copy link
Collaborator Author

Hello! Can you please provide steps to test/review the changes? I tried changing the rowActionSize at: https://deploy-preview-2874--moduswebcomponents.netlify.app/?path=/story/components-table--checkbox-row-selection&args=rowActionSize:99999 but it didn't appear to have any effect.

The example controls for rowActions are not added in the link provided, try the Row Actions type where the examples are added and rowActions have some example controls.
https://deploy-preview-2874--moduswebcomponents.netlify.app/?path=/story/components-table--row-actions

@ElishaSamPeterPrabhu ElishaSamPeterPrabhu force-pushed the 1922-table-uneven-width-on-row-selection-and-row-action-column branch from da9d990 to 66ea43d Compare September 20, 2024 05:39
@coliff
Copy link
Member

coliff commented Sep 20, 2024

These changes introduce new accessibility issues - can you please be sure to resolve them.
image

@ElishaSamPeterPrabhu
Copy link
Collaborator Author

These changes introduce new accessibility issues - can you please be sure to resolve them. image

image
These accessibility issues are present in the components web site as well as the sticky-right column does not have a text for its table header and the icon images of the row actions does not have alternate text

@ElishaSamPeterPrabhu ElishaSamPeterPrabhu force-pushed the 1922-table-uneven-width-on-row-selection-and-row-action-column branch from 905ccdc to 22c3f2b Compare September 25, 2024 06:08
@prashanth-offcl
Copy link
Contributor

@ElishaSamPeterPrabhu I can reproduce the same issue when using fullwidth to true. It might be due to the width set for other th's using getSize method, please check.

@ElishaSamPeterPrabhu
Copy link
Collaborator Author

@ElishaSamPeterPrabhu I can reproduce the same issue when using fullwidth to true. It might be due to the width set for other th's using getSize method, please check.

Sure, will check

@ElishaSamPeterPrabhu
Copy link
Collaborator Author

I attempted the following solutions, but either the row action column's width, which is set, does not remain fixed, or the widths of the other columns are reduced when the row action column's width stays the same.

  • Applied a fixed width to the sticky-right column, which worked for ensuring it stayed constant but didn’t solve the issue of gaps when fewer columns were present.
  • Tried using flex-grow: 1 on non-sticky columns to evenly distribute space. However, this caused the table headers to misalign and stack since flex doesn't work well within table elements.
  • Set min-width and max-width for columns to manage their resizing behavior. While this helped control sizing, it didn’t fully resolve the issue of the sticky column maintaining its width when other columns shrank.

Copy link

@vetrivel1 vetrivel1 left a comment

Choose a reason for hiding this comment

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

File: stencil-workspace/src/components/modus-checkbox/modus-checkbox.scss (line 6)
Adding justify-content: center; to the host may affect the alignment of checkbox content, especially if the component is used in layouts expecting left alignment. Please verify that this change does not unintentionally break existing layouts or cause visual regressions in consumer applications.
File: stencil-workspace/src/components/modus-table/models/table-context.models.ts (line 57)
Adding new properties to the TableContext interface is fine, but ensure that all usages of this interface throughout the codebase are updated accordingly. If these properties are optional, consider marking them as such to avoid breaking existing consumers.
File: stencil-workspace/src/components/modus-table/modus-table.tsx (line 175)
The new rowActionSize prop is not marked as optional in the type definition, but the comment suggests it is. Consider using @prop() rowActionSize?: number; to avoid potential issues if the prop is not provided.
File: stencil-workspace/src/components/modus-table/modus-table.tsx (line 524)
Passing rowActionSize through context is appropriate, but ensure that all consumers of this context property handle the case where it may be undefined.
File: stencil-workspace/src/components/modus-table/parts/modus-table-header.tsx (line 36)
The logic for rowActionsLength now allows for a custom size, but if rowActionSize is 0 or a falsy value, it will fall back to the calculated default. If 0 is a valid value, this may not be the intended behavior. Consider explicitly checking for undefined or null instead of falsy values.
File: stencil-workspace/src/components/modus-table/parts/row/selection/modus-table-header-checkbox.tsx (line 18)
The use of a Map for density widths is a good approach, but ensure that all possible density values are accounted for to avoid unexpected fallbacks to '46px'.

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.

Table: Uneven width on row selection and row action column
4 participants