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

[Resolve Errors] [Review Warnings] Tables - selectively enable borders based on if the table is overflowing #547

Closed
shindigira opened this issue May 20, 2024 · 3 comments · Fixed by #583
Labels
post-mvp to be done after MVP deadline

Comments

@shindigira
Copy link
Contributor

shindigira commented May 20, 2024

  • useIsOverflowing hook already added to the app
  • Requires ref forwarding on the Table container
@shindigira shindigira added the post-mvp to be done after MVP deadline label May 20, 2024
@natalia-fitzgerald
Copy link

@shindigira @meissadia
What if instead of doing this we removed the top, left, and right borders from the scrollable tables? I would want to only include the full border on the last page of paginated tables.

@shindigira
Copy link
Contributor Author

shindigira commented May 25, 2024

@shindigira @meissadia What if instead of doing this we removed the top, left, and right borders from the scrollable tables? I would want to only include the full border on the last page of paginated tables.

@natalia-fitzgerald What is the main UX aspect we are trying to convey?

Having a certain borders styling which will emphasize that the table is overflowing and thus horizontal scrolling is enabled?

@natalia-fitzgerald
Copy link

Yes, that is the aspect that any visual styling difference would be meant to convey. This is why it isn't ideal when tables that don't require horizontal scrolling receive this bordered styling. Generally I prefer the open styling of no borders (standard table styling). Since our app has the need for some horizontal scrolling tables we end up with visual inconsistency on the page. I'd like to have the horizontal scroll styling (borders) affect only the tables that require it.

shindigira added a commit to cfpb/design-system-react that referenced this issue May 28, 2024
closes #359

## Changes
- Adds ref forwarding
- adds a `divRef` and a `tableRef`

## How to test
- yarn test Table`

Reference
cfpb/sbl-frontend#547
shindigira added a commit that referenced this issue May 28, 2024
…ow when overflowing (horizontal scrolling active) (#583)

closes #547 

Note: Requires cfpb/design-system-react#358 to
be merged in first

## Changes
- enhancement-style(tables): full borders will only show for overflowing
- chore(tables): removed the need for `showTableBorders`

## How to test this PR

1. `yarn install` to get the latest DSR update with the table ref change
2. use
[all-errors-warnings-no-syntax-errors.csv](https://github.com/cfpb/sbl-frontend/files/15444623/all-errors-warnings-no-syntax-errors.csv)
or any register with multifield errors that causes
overflowing/horizontal scrolling
3. Verify that tables without overflowing don't have full borders,
tables that do have overflowing/horizontal scrolling have full borders

## Screenshot
<img width="638" alt="Screenshot 2024-05-25 at 11 47 57 AM"
src="https://github.com/cfpb/sbl-frontend/assets/13324863/635e1840-4ef4-4e86-8fc9-679f62cc3903">
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
post-mvp to be done after MVP deadline
Projects
None yet
2 participants