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.

Sign up for GitHub

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

Jump to bottom

Introduce Kirby Data Table #2517

Merged
merged 34 commits into from
Oct 17, 2022
Merged

Introduce Kirby Data Table #2517

merged 34 commits into from
Oct 17, 2022

Conversation

ghost
Copy link

@ghost ghost commented Oct 4, 2022 •
edited by ghost
Loading

Which issue does this PR close?

This PR closes #2494

What is the new behavior?

Added first iteration of the Data Table component.
Mostly a stylized html table.

Does this PR introduce a breaking change?

  • Yes
  • No

Are there any additional context?

As per: #2494

Checklist:

The following tasks should be carried out in sequence in order to follow the process of contributing correctly.

Reminders

Reg. tests, these have not been written yet, but i would like to see what everyone thinks of the implementation method.

  • Make sure you have implemented tests following the guidelines in: "The good: Test".
  • Make sure you have updated the cookbook with examples and showcases (for bug fixes, enhancements & new components).

Review

  • Do a self-review.
  • Request that the changes are code-reviewed
  • Request that the changes are UX reviewed (only necessary if your PR introduces visual changes)

When the pull request has been approved it will be merged to develop by Team Kirby.

@ghost ghost requested review from nmols, RasmusKjeldgaard and bduphl October 4, 2022 07:42
@github-actions github-actions bot temporarily deployed to pr-2494-kirby-data-table October 4, 2022 14:07 Inactive
RasmusKjeldgaard
RasmusKjeldgaard requested changes Oct 5, 2022
View reviewed changes
Copy link
Collaborator

@RasmusKjeldgaard RasmusKjeldgaard left a comment

Choose a reason for hiding this comment

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

Great work!

Here are my initial thoughts, I'll go through the scss a bit more thoroughly after we have discussed the overall structure - see comments on that.

@github-actions github-actions bot temporarily deployed to pr-2494-kirby-data-table October 6, 2022 07:47 Inactive
@github-actions github-actions bot temporarily deployed to pr-2494-kirby-data-table October 6, 2022 07:55 Inactive
@github-actions github-actions bot temporarily deployed to pr-2494-kirby-data-table October 6, 2022 10:34 Inactive
NillerW
NillerW previously approved these changes Oct 7, 2022
View reviewed changes
Copy link
Collaborator

@NillerW NillerW left a comment

Choose a reason for hiding this comment

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

Looks very good, just a minor comment to be made.

Would have loved to see a screenshot of the data-table-example attached to the PR :)

@RasmusKjeldgaard
Copy link
Collaborator

RasmusKjeldgaard commented Oct 7, 2022 •
edited
Loading

Looks very good, just a minor comment to be made.

Would have loved to see a screenshot of the data-table-example attached to the PR :)

You are probably aware of this @NillerW, but there is a deployed version of this available for inspection: https://cookbook.kirby.design/branch/kby-2494-kirby-data-table/#/home/showcase/data-table

@github-actions github-actions bot temporarily deployed to pr-2494-kirby-data-table October 10, 2022 06:15 Inactive
@SigurdVilstrup
implements onPush change detection
cbaf182 
- onPush change detection on Table Component
- onPush change detection on Table Component
- import fix in Data table module
@SigurdVilstrup
update table component unittest
f9b7204
@github-actions github-actions bot temporarily deployed to pr-2494-kirby-data-table October 12, 2022 12:24 Inactive
@github-actions github-actions bot temporarily deployed to pr-2494-kirby-data-table October 12, 2022 12:29 Inactive
@ghost ghost marked this pull request as ready for review October 12, 2022 12:30
@ghost ghost requested a review from jkaltoft as a code owner October 12, 2022 12:30
@github-actions github-actions bot temporarily deployed to pr-2494-kirby-data-table October 12, 2022 12:44 Inactive
@github-actions github-actions bot temporarily deployed to pr-2494-kirby-data-table October 12, 2022 12:46 Inactive
@SigurdVilstrup
remove view encapsulation
9d42445 
- removed view encapsulation
- removed th selection class
- moved kirby-selectable-row to table main scss
@SigurdVilstrup
updated showcase card example subheading
1fd32d8
@github-actions github-actions bot temporarily deployed to pr-2494-kirby-data-table October 13, 2022 15:05 Inactive
@github-actions github-actions bot temporarily deployed to pr-2494-kirby-data-table October 13, 2022 15:08 Inactive
@github-actions github-actions bot temporarily deployed to pr-2494-kirby-data-table October 14, 2022 10:46 Inactive
RasmusKjeldgaard
RasmusKjeldgaard requested changes Oct 14, 2022
View reviewed changes
@RasmusKjeldgaard
Copy link
Collaborator

RasmusKjeldgaard commented Oct 14, 2022 •
edited
Loading

A final set of suggestions for you @sigurdpvavilstrup. We are so close now, hang in there!

And big thanks to @jkaltoft for helping with the review, he gave us some great suggestions for the CSS.

@github-actions github-actions bot temporarily deployed to pr-2494-kirby-data-table October 16, 2022 19:05 Inactive
@github-actions github-actions bot temporarily deployed to pr-2494-kirby-data-table October 16, 2022 19:14 Inactive
RasmusKjeldgaard
RasmusKjeldgaard approved these changes Oct 17, 2022
View reviewed changes
Copy link
Collaborator

@RasmusKjeldgaard RasmusKjeldgaard left a comment

Choose a reason for hiding this comment

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

Thank you very much for your contribution. I love the end product, this is a great foundation for the coming work.

@RasmusKjeldgaard RasmusKjeldgaard changed the title Component/2494 kirby data table Introduce Kirby Data Table Oct 17, 2022
@RasmusKjeldgaard RasmusKjeldgaard merged commit e19c13f into develop Oct 17, 2022
@RasmusKjeldgaard RasmusKjeldgaard deleted the component/2494-kirby-data-table branch October 17, 2022 14:16
@RasmusKjeldgaard RasmusKjeldgaard mentioned this pull request Oct 28, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nmols nmols Awaiting requested review from nmols

@bduphl bduphl Awaiting requested review from bduphl

@jkaltoft jkaltoft Awaiting requested review from jkaltoft jkaltoft is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Component] Kirby Data Table
4 participants
@RasmusKjeldgaard @bdymgv @NillerW @SigurdVilstrup