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: implement enumerate rows to table component #1582

Conversation

ghost
Copy link

@ghost ghost commented May 23, 2020

feat: implement enumerate rows to table component

@commit-lint
Copy link

commit-lint bot commented May 23, 2020

Features

  • implement enumerate rows to table component (0ec0122)
  • specs added for enumerable column integration for Table component (0c4abed)

Bug Fixes

  • improved extra columns append to columnsData (5bb4827)
  • improved getColumns setup (f7bc1cb)
  • updated to align with spec results (b60d228)
  • counter updates and markup fix (fff845f)
  • counter (038f638)
  • firt data column (efe518c)

Contributors

@blakmetall, @LeandroTorresSicilia

@ghost ghost linked an issue May 23, 2020 that may be closed by this pull request
2 tasks
@ghost ghost requested a review from yvmunayev May 24, 2020 18:49
@LeandroTorresSicilia
Copy link
Member

LeandroTorresSicilia commented May 25, 2020

If you inspect an example with selection you will see this:
Screen Shot 2020-05-25 at 9 37 43 AM
it means that the first element in the row is a td tag with role="gridcell" (the column with the checkbox) and the second is a th tag with scope="row" (the first column with real content data)
In the case of the number column we should do the same
We should add unit test to this, to ensure that in the case where you use two columns before the data like using numbers and selection columns, then the third element in the row is which use the th tag and the scope="row" and the others are td elements with role="gridcell"

@ghost ghost closed this May 25, 2020
@ghost
Copy link
Author

ghost commented May 25, 2020

If you inspect an example with selection you will see this:
Screen Shot 2020-05-25 at 9 37 43 AM
it means that the first element in the row is a td tag with role="gridcell" (the column with the checkbox) and the second is a th tag with scope="row" (the first column with real content data)
In the case of the number column we should do the same
We should add unit test to this, to ensure that in the case where you use two columns before the data like using numbers and selection columns, then the third element in the row is which use the th tag and the scope="row" and the others are td elements with role="gridcell"

...updating markup and adding test cases on this

@LeandroTorresSicilia
Copy link
Member

LeandroTorresSicilia commented May 25, 2020

@blakmetall why you close this pr?
we shouldn't close PRs unnecessarily

@ghost
Copy link
Author

ghost commented May 25, 2020

@blakmetall why you close this pr?
we shouldn't close PRs unnecessarily

by mistake I believe, I wanted to delete a comment

@ghost ghost reopened this May 25, 2020
* css counter used for enumerable
* markup aligned with standards
@ghost
Copy link
Author

ghost commented May 25, 2020

SPECS DEFINITIONS PROPOSAL


Table

  • should not add a column when showCheckboxColumn and showRowNumberColumn are not passed (UPDATE)... previously: should not add a column when showCheckboxColumn is not passed
  • should add a column when showRowNumberColumn is passed (NEW)
  • should have the right value passed for enumerable column when showRowNumberColumn and rowNumberOffset are passed (NEW)
  • should add two columns when showCheckboxColumn and showRowNumberColumn are passed (NEW)
  • should update the columns state when add a column and (showCheckboxColumn and showRowNumberColumn) are not passed (UPDATE) ...previously:
    should update the columns state when add a column showCheckboxColumn is not passed
  • should update the columns state when add a column and showRowNumberColumn is passed (NEW)

body/cell.js

<Cell /> when isFirst is not passed

  • should render the enumerableCell component when columnType is "WITH_ENUMERABLE" (NEW)

<Cell /> when isFirst is is passed

  • should render the SelectableCell component when columnType is "WITH_ENUMERABLE" (NEW)

body/row.js

  • should set the right value to isFirst prop in the rendered Cell components when the first column contains the configurable checkbox selection column data (NEW)
  • should set the right value to isFirst prop in the rendered Cell components when the first column contains the configurable enumerable column data (NEW)
  • should set the right value to isFirst prop in the rendered Cell components when the first two columns contains the configurable enumerable and checkbox selection column data (NEW)

helpers/columns/getColumns.js

  • should return null when children is null, showCheckboxColumn is false and showRowNumberColumn is false (UPDATE)
  • should return an empty array when children is not passed, showCheckboxColumn is false
    and showRowNumberColumn is false (UPDATE)
  • should return an empty array when children is an array with falsy values, showCheckboxColumn is false and showRowNumberColumn is false (UPDATE)
  • should return an array with the columns props when showCheckboxColumn is false and showRowNumberColumn is false (UPDATE)
  • should return an array with the columns props, plus the selectable column when showCheckboxColumn is true (UNCHANGED)
  • should return an array with the columns props, plus the enumerable column data including enumerable offset value when showRowNumberColumn is true and rowNumberOffset is set (NEW)
    (the spec below was applied in the spec above)
    should return an array with the enumerable column containing the right rowNumberOffset value when showRowNumberColumn is true and rowNumberOffset is passed
  • should return an array with the columns props, plus the enumerable and the selectable columns when showCheckboxColumn and showRowNumberColumn are true (NEW)
  • should return an array with the right columns props when one column is type "action" (UNCHANGED)
  • should return an array with the right columns props when defaultWidth and width are passed (UNCHANGED)
  • should return an array with the right columns props when defaultWidth is passed and is out of range (UNCHANGED)

helpers/columns/getEnumerableWidth.js (NEW)

(moved as helper because it was needed to be mocked for a few specks)

  • should return the default width when value is not sent
  • should return the default calculated width when value is default
  • should return the right width when value length is 3 (999)

@LeandroTorresSicilia LeandroTorresSicilia merged commit 189bc64 into master Jun 1, 2020
@LeandroTorresSicilia LeandroTorresSicilia deleted the feat-implement-enumerate-rows-to-the-able-component branch June 1, 2020 21:13
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.

feat: implement enumerate rows to the Table component
3 participants