Skip to content
This repository has been archived by the owner on Jan 20, 2022. It is now read-only.

[AP-539] Table Additions #32

Merged
merged 4 commits into from
Aug 5, 2019
Merged

Conversation

fmvfelipemonsalve
Copy link
Contributor

@fmvfelipemonsalve fmvfelipemonsalve commented Jul 29, 2019

AP-539
This adds ability to set column widths for tables, plus added the loading state as spacified in the design spec. I made this modifications so I could finish AP-539.

@mayakoneval
Copy link
Contributor

@fmvfelipemonsalve do you mind if I add a commit to delete the left padding on the first column and the right padding on the last child? I noticed this is inconsistent with the designs we have for tables and I would like to slip it in here

@fmvfelipemonsalve
Copy link
Contributor Author

@mayakoneval Go for it. I did notice that, but didn't think it was important. I might need to make slight adjustments to the service tag tables after this, so let me know when you do this.

Copy link
Contributor

@justinanastos justinanastos left a comment

Choose a reason for hiding this comment

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

Let's talk about the changes I suggested! My goal is not just to keep this project easy to use but also to get you to think the same way :)

src/Table.tsx Outdated Show resolved Hide resolved
src/Table.tsx Outdated Show resolved Hide resolved
src/Table.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@mayakoneval mayakoneval left a comment

Choose a reason for hiding this comment

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

see comments!

@fmvfelipemonsalve fmvfelipemonsalve force-pushed the felipe/table-additions branch 4 times, most recently from b623020 to b715f62 Compare July 31, 2019 01:07
@fmvfelipemonsalve fmvfelipemonsalve changed the title [AP-539](https://golinks.io/j/AP-539) Felipe/table additions [AP-539] Felipe/table additions Jul 31, 2019
@fmvfelipemonsalve fmvfelipemonsalve changed the title [AP-539] Felipe/table additions [AP-539] Table Additions Jul 31, 2019
Copy link
Contributor

@mayakoneval mayakoneval left a comment

Choose a reason for hiding this comment

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

@fmvfelipemonsalve I might have messed something up when I pushed, but I see the old Table.tsx (src/Table.tsx rather than src/Table/Table.tsx) in the commits, so we should delete that if you think everything else is okay.

I also think we should definitely add a storybook showing a table that is configured with special column widths / spans something.

I added a commit to make the first column have no left padding and last column to have to right padding. Thoughts?

@fmvfelipemonsalve
Copy link
Contributor Author

My guess is you were working on an old branch, since I saw the old file yesterday and removed it. Will force push again to fix the branch (with your commit on top), and you can pull. Sounds good?

Copy link
Contributor

@justinanastos justinanastos left a comment

Choose a reason for hiding this comment

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

Love this! Almost there!

Please update the changelog too!

src/Table/Table.tsx Outdated Show resolved Hide resolved
src/Table/Table.tsx Show resolved Hide resolved
Copy link
Contributor

@justinanastos justinanastos left a comment

Choose a reason for hiding this comment

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

LGTM!

@fmvfelipemonsalve fmvfelipemonsalve merged commit a4226b4 into master Aug 5, 2019
@fmvfelipemonsalve fmvfelipemonsalve deleted the felipe/table-additions branch August 5, 2019 20:45
@fmvfelipemonsalve fmvfelipemonsalve restored the felipe/table-additions branch August 6, 2019 00:13
@fmvfelipemonsalve fmvfelipemonsalve deleted the felipe/table-additions branch August 6, 2019 00:33
@justinanastos justinanastos added patch minor Increment the minor version when merged released This issue/pull request has been released. and removed minor Increment the minor version when merged patch labels Aug 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
minor Increment the minor version when merged released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants