-
Notifications
You must be signed in to change notification settings - Fork 12
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 selecting rows #344
Conversation
✅ Deploy Preview for sefirot-story ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for sefirot-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #344 +/- ##
==========================================
- Coverage 84.99% 84.84% -0.16%
==========================================
Files 139 142 +3
Lines 11192 11374 +182
Branches 640 515 -125
==========================================
+ Hits 9513 9650 +137
- Misses 1679 1724 +45
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made few comments, but overall, I love it! very simple to use with good API 🎉
@brc-dd I've adjusted the style of the actions. I wanted to make it look like this. So I also made "Reset filters" a button look as well. Then I realized now we don't need divider since button acts like it 👀 So, I divided stats and actions array. Also changed the text of total records. Removed Could you double check if I didn't break anything 🙏 The functionality is working really smooth! |
Working fine for me 🙌 |
Should we deprecate the hasFilters + onReset? 👀 Like ignore that when actions is specified, so any new code should just pass that directly as action instead of passing through all those components? Might seem repetitive though, but would be more consistent. |
Ah, Good point! You're right now we don't need it 👀 Let's mark it as deprecated 👍 Maybe adding comments to Table composable...? |
We would have |
note regarding the border fixes - https://cloudfour.com/thinks/the-math-behind-nesting-rounded-corners/ (we have a border radius on .box and then we have nested border radius inside header/body/footer) however, in our case due to 1px border width, it doesn't affect that much and one won't see the gap without zooming. also, now everything respects that border-radius variable: Before: After: PS: in button group, |
Ah, thanks! |
API:
TODO:
Preview permalink: https://650952c866a4ec00081d0374--sefirot-story.netlify.app/story/stories-components-stable-01-playground-story-vue?variantId=_default