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

Feature/data table striped #1591

Merged
merged 4 commits into from
Nov 15, 2021

Conversation

JerryChan31
Copy link
Contributor

@JerryChan31 JerryChan31 commented Nov 15, 2021

implementation of #1552.

Something may need to be discussed:

  1. I'm not sure if it's proper to use var(--td-color-hover) as stripe color.
  2. I'm using just css to implement this feature, but it seems not possible to test it with Jest. Is there a better way?

It's my first PR to open source community so if there's any problem just point it out and I'll fix it.

@vercel
Copy link

vercel bot commented Nov 15, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/tusimple/naive-ui/GDJ6UXrjMU6GWGfVRMY7seBco4TQ
✅ Preview: https://naive-ui-git-fork-jerrychan31-feature-data-tabl-6277d1-tusimple.vercel.app

Comment on lines +306 to +308
cM('striped', [
cB('data-table-tr:nth-child(even)', [cB('data-table-td', 'background-color: var(--td-color-hover);')])
]),
Copy link
Collaborator

Choose a reason for hiding this comment

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

In fact this is not the correct behavior since when table row is expandable, the even odd status of the row will be changed.

However it won't be hard since we can just follow the index to distinguish the row status.

@codecov
Copy link

codecov bot commented Nov 15, 2021

Codecov Report

Merging #1591 (1ecec15) into main (beddc93) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1591   +/-   ##
=======================================
  Coverage   64.26%   64.26%           
=======================================
  Files         885      885           
  Lines       17130    17130           
  Branches     4050     4050           
=======================================
  Hits        11008    11008           
  Misses       5382     5382           
  Partials      740      740           
Impacted Files Coverage Δ
src/data-table/src/DataTable.tsx 77.33% <ø> (ø)
src/data-table/src/styles/index.cssr.ts 100.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update beddc93...1ecec15. Read the comment docs.

@07akioni 07akioni merged commit 5e820cb into tusen-ai:main Nov 15, 2021
@07akioni
Copy link
Collaborator

I'm wrong, actually the refactor is not easy. hahaha.

See c522347 to know what is changed to make it work in all possible cases.

Ghjsw pushed a commit to Ghjsw/naive-ui that referenced this pull request Nov 16, 2021
* feat(data-table): striped data table.

* test(data-table): add striped test, only checking class.

* docs(CHANGELOG, demo): add en demo docs, add CHANGELOG

Co-authored-by: 07akioni <07akioni2@gmail.com>
Ghjsw pushed a commit to Ghjsw/naive-ui that referenced this pull request Nov 16, 2021
* feat(data-table): striped data table.

* test(data-table): add striped test, only checking class.

* docs(CHANGELOG, demo): add en demo docs, add CHANGELOG

Co-authored-by: 07akioni <07akioni2@gmail.com>
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.

2 participants