Skip to content
This repository has been archived by the owner on May 24, 2024. It is now read-only.

[terra-table] Rerganized Cell.jsx for easier maintenance & readability #2061

Merged
merged 4 commits into from
Mar 4, 2024

Conversation

sdadn
Copy link
Contributor

@sdadn sdadn commented Mar 4, 2024

Summary

This PR reorganizes the cell.jsx file for easier maintenance & readability by moving the code into labelled sections. There are no functional changes in this PR.

Testing

This change was tested using:

  • WDIO
  • Jest
  • Visual testing (please attach a screenshot or recording)
  • Other (please describe below)
  • No tests are needed

Reviews

In addition to engineering reviews, this PR needs:

  • UX review
  • Accessibility review
  • Functional review

Additional Details


Thank you for contributing to Terra.
@cerner/terra

@sdadn sdadn self-assigned this Mar 4, 2024
@github-actions github-actions bot temporarily deployed to preview-pr-2061 March 4, 2024 16:53 Destroyed
@github-actions github-actions bot temporarily deployed to preview-pr-2061 March 4, 2024 17:00 Destroyed
Copy link
Contributor

github-actions bot commented Mar 4, 2024

Fails
🚫 Please include a CHANGELOG entry for each changed package this PR. Looks like a CHANGELOG entry is missing for:
  • terra-table

Generated by 🚫 dangerJS against 83b0a5c

@github-actions github-actions bot temporarily deployed to preview-pr-2061 March 4, 2024 17:07 Destroyed
@sdadn
Copy link
Contributor Author

sdadn commented Mar 4, 2024

Fails
🚫 Please include a CHANGELOG entry for each changed package this PR. Looks like a CHANGELOG entry is missing for:

  • terra-table

Generated by 🚫 dangerJS against 41de8bd

CHANGELOG updates are not needed.

@cm9361
Copy link
Contributor

cm9361 commented Mar 4, 2024

Are these changes necessary? It might make it harder to maintain to have manually added headers for code. I feel like some of the headers would make it confusing where to add code.

@sdadn
Copy link
Contributor Author

sdadn commented Mar 4, 2024

Test failures are due to other wdio components. The jest tests passing indicate that the cell.jsx is still functionally identical after this change.

@sdadn
Copy link
Contributor Author

sdadn commented Mar 4, 2024

Are these changes necessary? It might make it harder to maintain to have manually added headers for code. I feel like some of the headers would make it confusing where to add code.

@cm9361 The headers help to indicate what each block of code does at a glance, but I can remove them if it can cause more confusion in the future. Besides the headers, these changes are mainly grouping functionally similar code together to minimize jumping back and forth, such as putting all DOM related variables at the end.

@github-actions github-actions bot temporarily deployed to preview-pr-2061 March 4, 2024 19:34 Destroyed
@sdadn
Copy link
Contributor Author

sdadn commented Mar 4, 2024

@cm9361 Removed all comment headers here: 83b0a5c

@sdadn sdadn merged commit 228d12d into main Mar 4, 2024
20 of 22 checks passed
@sdadn sdadn deleted the table-cell-update branch March 4, 2024 20:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants