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.

Sign up for GitHub

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

Jump to bottom

Refactor Table styles and props with updated examples and tests. #231

Merged
merged 3 commits into from
May 12, 2016
Merged

Refactor Table styles and props with updated examples and tests. #231

merged 3 commits into from
May 12, 2016

Conversation

ogupte
Copy link
Contributor

@ogupte ogupte commented May 11, 2016 •
edited by jondlm
Loading

PR Checklist

  • Manually tested across supported browsers
    • Chrome
    • Firefox
    • Safari
    • IE11 (Win 7)
    • Edge (Win 10)
  • Unit tests written (common at minimum)
  • PR has one of the semver- labels
  • Two core team engineer approvals
  • One core team UX approval

Breaking Changes:

Table

Table.Thead

  • change className from lucid-Table-thead to lucid-Table-Thead

Table.Tbody

  • change className from lucid-Table-tbody to lucid-Table-Tbody

Table.Tr

  • change className from lucid-Table-row to lucid-Table-Tr
  • removed prop isHeader
  • renamed prop hasDetails to isActionable

Table.Th

  • change className from lucid-Table-cell to lucid-Table-Th
  • removed prop hasCheckbox
  • removed prop hasButton
  • removed prop hasIcon
  • updated the rendered output to have a more flex-driven layout to handle sorting icons and resizers

Table.Td

  • change className from lucid-Table-cell to lucid-Table-Td
  • removed prop isAfterRowSpan
  • removed prop hasCheckbox
  • removed prop hasButton
  • removed prop hasIcon

ScrollTable

  • renamed prop isNowrap to hasWordWrap

DataTable

  • removed prop hasExtraWhiteSpace
  • renamed props hasDetails to isActionable

@ogupte ogupte self-assigned this May 11, 2016
@ogupte ogupte added this to the 1.0.0 milestone May 11, 2016
ogupte
ogupte reviewed May 11, 2016
View reviewed changes
src/components/Table/Table.less
@@ -1,371 +1,299 @@
@Table-row-height: @size-standard-height - 2;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

these less variables need to be cleaned up, there are some unused vars defined here.

@jondlm
Copy link
Contributor

jondlm commented May 11, 2016

Might want to do an ag for hasExtraWhitespace. Looks like there are some left over in examples.

jondlm
jondlm reviewed May 11, 2016
View reviewed changes
src/components/Table/Table.jsx
*
* Returns a 2 dimensional array of cell elements of the given component type. The map function can modify value of a cell.
*/
const mapToGrid = function mapToGrid(trList, cellType='td', mapFn=_.property('element')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason this is a const and not just a regular function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

whoops looks like a fluke, i'll fix it

@jondlm jondlm mentioned this pull request May 11, 2016
Closed
2 tasks
sodiumjoe
sodiumjoe reviewed May 11, 2016
View reviewed changes
src/components/ScrollTable/examples/1.standard.jsx
@@ -34,140 +34,140 @@ export default React.createClass({
<Tbody>
<Tr>
<Td rowSpan={14} hasBorderRight>RS</Td>
<Td hasCheckbox><Checkbox/></Td>
Copy link
Contributor

Choose a reason for hiding this comment

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

nice

jondlm
jondlm reviewed May 11, 2016
View reviewed changes
src/components/Table/Table.less
@@ -1,371 +1,299 @@
@Table-row-height: @size-standard-height - 2;
@Table-row-heightLarge: @Table-row-height + @size-standard;
@Table-cell-padding: @size-S;
@Table-cell-border-color: #e3e3e3;
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be @color-gray

@ogupte ogupte merged commit 1c9df95 into release/1.0.0 May 12, 2016
@ogupte ogupte deleted the refactor/table-styles-2 branch May 12, 2016 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@ogupte ogupte

Labels
quality semver-major
Projects
None yet
Milestone
1.0.0
Development

Successfully merging this pull request may close these issues.
4 participants
@ogupte @jondlm @sodiumjoe @strunkandwhite