Skip to content

Conversation

@sroy3
Copy link
Contributor

@sroy3 sroy3 commented Apr 18, 2023

Don't want you to review this with the multiple branches in the table feature. Just cleaned up a little before adding the feature.

  • Moved some components inside a body subfolder in table.
  • Moved utils and slices out of the components folder`.
  • Moved the rows.map => TableBody to its own file (where the main change is going to happen later)
  • Split WorkspaceRowGroup from TableBody to its own component.

I could really go into the deep end, trying to fix the codeclimate issues and making smaller CSS modules, but I'll stop at this for now.

@sroy3 sroy3 self-assigned this Apr 18, 2023
@sroy3 sroy3 marked this pull request as ready for review April 18, 2023 18:41
Copy link
Contributor

@julieg18 julieg18 left a comment

Choose a reason for hiding this comment

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

Looks like there's some merge conficts due to #3667 being merged (sorry about that 😅). But besides that, looks good! Great work!

Copy link
Contributor

Choose a reason for hiding this comment

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

Should commitsAndBranches be outside of the table folder since it's below the table?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's being moved inside with the multiple branches stuff

@qlty-cloud-legacy
Copy link

Code Climate has analyzed commit d3630ec and detected 6 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 3
Duplication 3

The test coverage on the diff in this pull request is 97.8% (85% is the threshold).

This pull request will bring the total coverage in the repository to 94.8% (0.0% change).

View more on Code Climate.

@sroy3 sroy3 merged commit c60913f into main Apr 20, 2023
@sroy3 sroy3 deleted the table-hierarchy branch April 20, 2023 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants