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

feat(table): allow adding summary row #231

Merged
merged 8 commits into from
Mar 28, 2023
Merged

feat(table): allow adding summary row #231

merged 8 commits into from
Mar 28, 2023

Conversation

brc-dd
Copy link
Member

@brc-dd brc-dd commented Mar 27, 2023

fixes #230

  • Sorting and filtering does not affect the summary row (always shown).
  • Pagination is not working (at least not on Histoire), so not sure how it will play with that 👀
    • onPrev/onNext is empty there, will check if it works fine or not

Also, on filtering, there are a bunch of errors (not related to this PR):

image

TODO:

  • Update docs
  • Add tests

@netlify
Copy link

netlify bot commented Mar 27, 2023

Deploy Preview for sefirot-docs ready!

Name Link
🔨 Latest commit b3af0c9
🔍 Latest deploy log https://app.netlify.com/sites/sefirot-docs/deploys/642257e61760d10008bfdf35
😎 Deploy Preview https://deploy-preview-231--sefirot-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Mar 27, 2023

Deploy Preview for sefirot-story ready!

Name Link
🔨 Latest commit b3af0c9
🔍 Latest deploy log https://app.netlify.com/sites/sefirot-story/deploys/642257e6c0e9f60008dda0f7
😎 Deploy Preview https://deploy-preview-231--sefirot-story.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@codecov
Copy link

codecov bot commented Mar 27, 2023

Codecov Report

Patch coverage: 96.96% and project coverage change: -0.55 ⚠️

Comparison is base (cff18f1) 80.04% compared to head (b3af0c9) 79.50%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #231      +/-   ##
==========================================
- Coverage   80.04%   79.50%   -0.55%     
==========================================
  Files         112      112              
  Lines        9335     9366      +31     
  Branches      347      368      +21     
==========================================
- Hits         7472     7446      -26     
- Misses       1863     1920      +57     
Impacted Files Coverage Δ
lib/components/STable.vue 85.89% <92.85%> (+0.27%) ⬆️
lib/components/STableCell.vue 57.89% <100.00%> (-42.11%) ⬇️
lib/components/STableCellDay.vue 100.00% <100.00%> (ø)
lib/components/STableCellText.vue 94.90% <100.00%> (-5.10%) ⬇️
lib/composables/Table.ts 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@kiaking kiaking left a comment

Choose a reason for hiding this comment

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

Looking nice! Very smart 👍

lib/composables/Table.ts Outdated Show resolved Hide resolved
@kiaking
Copy link
Member

kiaking commented Mar 27, 2023

Pagination is not working (at least not on Histoire), so not sure how it will play with that 👀

  • onPrev/onNext is empty there, will check if it works fine or not

Yap! Currently updating records are manual so, if the summary should change on page change, it should be done at user land. Just pass reactive value to summary option and update it. Though, I think it's rare to use summary for table with pagination, but I don't see any issues.

Also, on filtering, there are a bunch of errors (not related to this PR):

hmmmm something happening on Histoire I guess 🤔 ...

Copy link
Member

@kiaking kiaking left a comment

Choose a reason for hiding this comment

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

Thanks a bunch! I've adjusted the style and defined new css var for font weight.

@kiaking kiaking merged commit e55ec58 into main Mar 28, 2023
@kiaking kiaking deleted the feat/table-summary branch March 28, 2023 03:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Table] Add summary feature
2 participants