Skip to content
This repository was archived by the owner on Jun 4, 2024. It is now read-only.

Styling props #165

Merged
merged 3 commits into from
Oct 24, 2018
Merged

Styling props #165

merged 3 commits into from
Oct 24, 2018

Conversation

Marc-Andre-Rivet
Copy link
Contributor

@Marc-Andre-Rivet Marc-Andre-Rivet commented Oct 24, 2018

  • additional props
  • generator functions for style, proptypes, py2js prop mapping

This PR is about adding support for the missing style CSS properties. Doing so increases the number of props for the various style objects from ~150 to ~750 and makes the size of the bundle explode (~330 -> ~730kB) if the dash typing remains as tight. Hence, reducing the amount of typing done at dash-level in favor of continued full sanitation in TS -- if/when a communication channel for errors and warnings becomes available between the FE components and the dash server, mismtaches could be communicated as warnings through that channel.

Not bumping up version as this only refines the work done in RC6.

- generator functions for style, proptypes, py2js prop mapping
@chriddyp chriddyp temporarily deployed to dash-table-review-pr-165 October 24, 2018 14:25 Inactive
@@ -72,7 +72,7 @@
"webpack": "^4.8.3",
"webpack-cli": "^2.1.3",
"webpack-dev-server": "^3.1.5",
"webpack-preprocessor": "^0.1.11"
"webpack-preprocessor": "^0.1.12"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previous version of the preprocessor was having issues with very large files. This one does not..

'writingMode',
'zIndex',
'zoom'
];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This list is not quite comprehensive but it's definitely more than the previous one..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file generates the various usages of the props -- just too easy to make a mistake modifying them manually


snakes.forEach(([snake, camel]) => map.set(snake, camel));
kebabs.forEach(([kebab, camel]) => map.set(kebab, camel));
camels.forEach(([camel]) => map.set(camel, camel));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Map the camel cased props above to kebab and snake

stream1.write('\n]);')

stream1.end();
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The camel/kebab/snake to camel mapping for react style

stream2.write('}')

stream2.end();
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The style interface for TS

stream3.write('\n')

stream3.end();
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

proptypes fragment for table.js / props definition

@chriddyp chriddyp temporarily deployed to dash-table-review-pr-165 October 24, 2018 15:24 Inactive
verticalAlign: PropTypes.oneOfType([PropTypes.string, PropTypes.number]),
zIndex: PropTypes.oneOfType([PropTypes.string, PropTypes.number])
}),
style_table: PropTypes.object,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Precise typing is too costly for the styles, will sanitize the props internally instead
(the build with all the explicit props increased by over 150%)

…itional-props

# Conflicts:
#	dash_table/bundle.js
#	dash_table/demo.js
Copy link
Contributor

@valentijnnieman valentijnnieman 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, but would love to know what @chriddyp thinks!

Copy link
Member

@chriddyp chriddyp left a comment

Choose a reason for hiding this comment

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

Fine by me. We'll have a way to display front-end errors to Dash devs soon, so we can just do validation there.
We also don't do any style validation for the rest of our components, so this isn't making it worse than anything else we provide.

@Marc-Andre-Rivet Marc-Andre-Rivet merged commit 56436e4 into master Oct 24, 2018
@Marc-Andre-Rivet Marc-Andre-Rivet deleted the 3.1-styling-api-additional-props branch July 18, 2019 12:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants