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

chore(flat-table-header): add displayName to component FE-5208 #5214

Merged
merged 5 commits into from
Jun 17, 2022

Conversation

edleeks87
Copy link
Contributor

@edleeks87 edleeks87 commented Jun 9, 2022

fix #5208

Proposed behaviour

Updates functionality to use child.type.displayName instead of child.type. This will allow
wrapping of the FlatTableRowHeader component as the wrapper component can set its displayName to
FlatTableRowHeader.displayName to ensure the required columns are sticky.

Adds displayName property to children of FlatTableRow.

Current behaviour

Wrapping the FlatTableRowHeader prevents sticky column functioning correctly
Children of FlatTableRow have no displayName property

Checklist

  • Commits follow our style guide
  • Related issues linked in commit messages if required
  • Unit tests added or updated if required
  • Storybook added or updated if required

QA

  • Tested in CodeSandbox/storybook
  • Add new Cypress test coverage if required
  • Carbon implementation matches Design System/designs
  • UI Tests GitHub check reviewed if required

Additional context

Testing instructions

Use sandbox below, the one included in the linked issue will not work as there is no way to dynamically set the displayName on the wrapper. I have also added a story to demo how to implement this flat-table--wrapping-row-headers

The following CodeSandbox is an example of the broken behaviour.
You can see the new behaviour by looking at the version in the comment by codesandbox[bot].

https://codesandbox.io/s/carbon-quickstart-forked-y63erp?file=/src/index.js

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jun 9, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit d15cb0d:

Sandbox Source
carbon-quickstart Configuration
carbon-quickstart-typescript Configuration
carbon-quickstart PR
carbon-quickstart (forked) PR
wispy-night-l5bykk Issue #5208

@cypress
Copy link

cypress bot commented Jun 9, 2022



Test summary

2215 0 2 0Flakiness 0


Run details

Project carbon
Status Passed
Commit d15cb0d
Started Jun 17, 2022 1:01 PM
Ended Jun 17, 2022 1:07 PM
Duration 05:30 💡
OS Linux Debian - 10.10
Browser Chrome 100

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@edleeks87 edleeks87 force-pushed the FE-5208-wrapping-sticky-col branch 5 times, most recently from 7434fc8 to 5ed77e7 Compare June 9, 2022 13:22
@edleeks87 edleeks87 changed the title chore: temp chore(flat-table-header): add displayName to component FE-5208 Jun 9, 2022
@edleeks87 edleeks87 force-pushed the FE-5208-wrapping-sticky-col branch from 5ed77e7 to 8042c45 Compare June 9, 2022 13:25
DipperTheDan
DipperTheDan previously approved these changes Jun 9, 2022
@edleeks87 edleeks87 self-assigned this Jun 13, 2022
Parsium
Parsium previously approved these changes Jun 15, 2022
@Parsium Parsium marked this pull request as ready for review June 15, 2022 13:22
@Parsium Parsium requested review from a team as code owners June 15, 2022 13:22
…apping row headers

Updates functionality to use `child.type.displayName` instead of `child.type`. This will allow
wrapping of the `FlatTableRowHeader` component as the wrapper component can set its `displayName` to
`FlatTableRowHeader.displayName` to ensure the required columns are sticky.

fix #5208
@edleeks87 edleeks87 merged commit a805622 into master Jun 17, 2022
@edleeks87 edleeks87 deleted the FE-5208-wrapping-sticky-col branch June 17, 2022 13:42
@carbonci
Copy link
Collaborator

🎉 This PR is included in version 107.1.6 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Sticky column with wrapper component is not working properly
5 participants