blackbaud / skyux2-docs

Documentation for SKY UX Components
4 stars 32 forks source link

Update grid docs to specify unit for value of column's width property #988

Closed blackbaud-johnly closed 4 years ago

blackbaud-johnly commented 4 years ago

Question came up on Slack about the property description in the docs that just says "Specifies the width of the column." and whether the value should be pixels, em, percentage, or any of those.

jhenderson2099 commented 4 years ago

May want to indicate that this must be a number.

That is, the grid column component cannot use something like "100%" which is not a number(from a JavaScript perspective. In addition, you also can't use CSS calc as that allows for XSS attacks. (You would need to bypass the security within the component and use the DomSanitizer to trust that.)

jhenderson2099 commented 4 years ago

One additional thing to document is what the behavior is within the component if this property is not specified or specified as undefined.

blackbaud-johnly commented 4 years ago

First part is addressed in https://github.com/blackbaud/skyux2-docs/pull/989. I didn't see the additional suggestions until after I merged that. I'll keep the issue open to address those points in another PR. It looks like the first comment about it being a number is already addressed given that the description already says "This property accepts number values." But I'll look into whether we need to add anything about CSS calc and also find out what the behavior is when this is not defined.

blackbaud-johnly commented 4 years ago

Addressed in https://github.com/blackbaud/skyux2-docs/pull/997.