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(b-table, b-table-lite): new `tbody-tr-attr` prop for arbitrary row attributes (closes #1864) #4481

Merged
merged 17 commits into from Dec 10, 2019

Conversation

@CyBot
Copy link
Contributor

CyBot commented Dec 9, 2019

Adds new prop tbody-tr-attr to b-table and b-table-lite, which can either be an object or a function
An object is added straight to the attributes of each row
A function gets passed each item, and row type, and is expected to return an object

The function version has the same calling signature as the tbody-tr-class prop.

Describe the PR

Allows to add custom attributes to individual table rows, either directly as name: value object attributes, or via a function which receives the row item and returns such an object.

Closes #1864

PR checklist

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Enhancement
  • ARIA accessibility
  • Documentation update
  • Other (please describe)

Does this PR introduce a breaking change? (check one)

  • No
  • Yes (please describe)

The PR fulfills these requirements:

  • It's submitted to the dev branch, not the master branch
  • When resolving a specific issue, it's referenced in the PR's title (i.e. [...] (fixes #xxx[,#xxx]), where "xxx" is the issue number)
  • It should address only one issue or feature. If adding multiple features or fixing a bug and adding a new feature, break them into separate PRs if at all possible.
  • The title should follow the Conventional Commits naming convention (i.e. fix(alert): not alerting during SSR render, docs(badge): update pill examples, chore(docs): fix typo in README, etc). This is very important, as the CHANGELOG is generated from these messages.

If new features/enhancement/fixes are added or changed:

  • Includes documentation updates (including updating the component's package.json for slot and event changes)
  • Includes any needed TypeScript declaration file updates
  • New/updated tests are included and passing (if required)
  • Existing test suites are passing
  • CodeCov for patch has met target
  • The changes have not impacted the functionality of other components or directives
  • ARIA Accessibility has been taken into consideration (Does it affect screen reader users or keyboard only users? Clickable items should be in the tab index, etc.)

If adding a new feature, or changing the functionality of an existing feature, the PR's
description above includes:

  • A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)
Adds a trAttr attribute to b-table, which can either be an object or a function
An object is added straight to the attributes of each row
A function gets passed each item and is expected to return an object
@codecov

This comment has been minimized.

Copy link

codecov bot commented Dec 9, 2019

Codecov Report

Merging #4481 into dev will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #4481      +/-   ##
==========================================
+ Coverage   99.91%   99.91%   +<.01%     
==========================================
  Files         247      247              
  Lines        4851     4855       +4     
  Branches     1359     1363       +4     
==========================================
+ Hits         4847     4851       +4     
  Misses          3        3              
  Partials        1        1
Impacted Files Coverage Δ
src/components/table/helpers/mixin-bottom-row.js 100% <ø> (ø) ⬆️
src/components/table/helpers/mixin-busy.js 100% <ø> (ø) ⬆️
src/components/table/helpers/mixin-empty.js 100% <ø> (ø) ⬆️
src/components/table/helpers/mixin-top-row.js 100% <ø> (ø) ⬆️
src/components/table/helpers/mixin-tbody-row.js 100% <100%> (ø) ⬆️

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 4abe675...703d2e3. Read the comment docs.

CyBot added 2 commits Dec 9, 2019
Missed second one...
@tmorehouse tmorehouse changed the title feat(b-table): add custom row attributes feat(b-table, b-table-lite): new `tr-attr` prop for arbitrary row attributes Dec 9, 2019
@tmorehouse

This comment was marked as resolved.

Copy link
Member

tmorehouse commented Dec 9, 2019

I am wondering if this prop should be tbody-tr-attr, since unlike tr-class, this prop only applied to rows in the body (tr-class also is used for rows in the head, foot, and other locations within the table)

As well, perhaps mention in the prop descriptions that the attributes only get applied to item row <tr> elements.

Unless this was modified to work like tbody-tr-class where it is called for all various tr elements and is passed a second argument which specifies the type of row it is being applied to (i.e. row, details row, fixed-top, empty-row, etc)

@CyBot

This comment was marked as resolved.

Copy link
Contributor Author

CyBot commented Dec 9, 2019

I am wondering if this prop should be tbody-tr-attr, since unlike tr-class, this prop only applied to rows in the body (tr-class also is used for rows in the head, foot, and other locations within the table)

Whatever better suits your nomenclature, I'm pretty new to BootstrapVue

@tmorehouse

This comment was marked as resolved.

Copy link
Member

tmorehouse commented Dec 9, 2019

I'm now thinking that this might be better mimicking how tbody-tr-class works (with the second argument), and have it also added to all other places we generate <tr> elements in the table body, as i know people will expect it to work similar to tbody-tr-class to apply to any type of row in the body

tmorehouse added 10 commits Dec 10, 2019
@tmorehouse

This comment has been minimized.

Copy link
Member

tmorehouse commented Dec 10, 2019

I've tweaked the PR to make the prop tbody-tr-attr, with the same calling signature as tbody-tr-class

src/components/table/index.d.ts Outdated Show resolved Hide resolved
@tmorehouse tmorehouse changed the title feat(b-table, b-table-lite): new `tr-attr` prop for arbitrary row attributes feat(b-table, b-table-lite): new `tbody-tr-attr` prop for arbitrary row attributes Dec 10, 2019
@tmorehouse tmorehouse changed the title feat(b-table, b-table-lite): new `tbody-tr-attr` prop for arbitrary row attributes feat(b-table, b-table-lite): new `tbody-tr-attr` prop for arbitrary row attributes (closes #1864) Dec 10, 2019
@tmorehouse tmorehouse added this to In progress in 2.2.0 via automation Dec 10, 2019
@tmorehouse

This comment has been minimized.

Copy link
Member

tmorehouse commented Dec 10, 2019

@CyBot can you take another look to make sure I don't have any other typos creeping in?

And thanks for the PR, this has been on the back-burner for a while.

@tmorehouse tmorehouse requested a review from jackmu95 Dec 10, 2019
@CyBot

This comment has been minimized.

Copy link
Contributor Author

CyBot commented Dec 10, 2019

@CyBot can you take another look to make sure I don't have any other typos creeping in?

Looking good

@jackmu95 jackmu95 merged commit 4acf6ed into bootstrap-vue:dev Dec 10, 2019
8 checks passed
8 checks passed
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: setup Your tests passed on CircleCI!
Details
ci/circleci: test Your tests passed on CircleCI!
Details
codecov/patch 100% of diff hit (target 99.91%)
Details
codecov/project 99.91% (+<.01%) compared to 4abe675
Details
deploy/netlify Deploy preview ready!
Details
security/snyk - package.json (pi0) No manifest changes detected
2.2.0 automation moved this from In progress to Done Dec 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
2.2.0
  
Done
3 participants
You can’t perform that action at this time.