Skip to content

Renamed Page/Drawer sub components#80

Merged
oilywithraybans merged 14 commits intodevfrom
feature/rename-data-grid
Oct 28, 2019
Merged

Renamed Page/Drawer sub components#80
oilywithraybans merged 14 commits intodevfrom
feature/rename-data-grid

Conversation

@oilywithraybans
Copy link
Copy Markdown

@oilywithraybans oilywithraybans commented Oct 25, 2019

  • Page/Drawer Details -> DetailsWindow
  • Page/Drawer Grid -> DataCards
  • Page/Drawer Table -> DataGrid

@oilywithraybans oilywithraybans changed the title Renamed Page/Drawer DataGrid sub component to DataCard Renamed Page/Drawer sub components Oct 25, 2019
@groberts314
Copy link
Copy Markdown
Contributor

@morethanfire Looks like some overlap here with #79?

@oilywithraybans oilywithraybans added the WIP Work In Progress label Oct 25, 2019
Copy link
Copy Markdown
Contributor

@groberts314 groberts314 left a comment

Choose a reason for hiding this comment

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

Looks good to me so far ... but seems still WIP?

Page.Grid = PageGrid;
Page.Table = PageTable;
Page.Grid = PageDataCards; // TODO: Deprecated. Alias name for Page.DataCard. Remove in next major release.
Page.Table = PageDataGrid; // TODO: Deprecated. Alias name for Page.DataGrid. Remove in next major release.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How widespread is the usage in HC? Is it worth deprecating? Or should we kill altogether and replace all the references in HC?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Not to wide. I will fix in dev and let everyone know of a possible regression and what to do for their branches. In my eyes, super simple fix.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

But you'd rather leave the old aliases just in case?
Honestly, I'm pretty certain we're the only consumers ... I'd almost rather remove, let it fail hard until fixed. Not the way we'd do things if it really were a shared library, and if you do want to do the deprecation and them remove in next major version, I am okay with that too.

@oilywithraybans oilywithraybans removed the WIP Work In Progress label Oct 28, 2019
},
]}
data={{
birthday: '23/01/1990',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we do 1990-01-23 for a common format intelligible and agreeable to devs all over the world? =)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

changed to this and also added the accessor function statement

row: PropTypes.shape({}).isRequired,
rowIndex: PropTypes.number.isRequired,
rowProps: PropTypes.func,
sizes: PropTypes.arrayOf(PropTypes.arrayOf(PropTypes.object)),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Uh oh, PropTypes.object! Isn't that verboten these days per ESLINT? It's PropTypes.shape({}) now? =)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

fixed.

import Header from '../elements/header.js';
import List from '../elements/list.js';
import DataGroupRow from './dataGroupRow.js';
import DataGroupRow from './DataGroupRow.js';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Uh, why?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

fixed.

Copy link
Copy Markdown
Contributor

@groberts314 groberts314 left a comment

Choose a reason for hiding this comment

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

Looks great! :shipit:

@oilywithraybans oilywithraybans merged commit 63699e9 into dev Oct 28, 2019
@groberts314 groberts314 deleted the feature/rename-data-grid branch December 13, 2019 21:44
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