Conversation
src/components/loading/Loading.tsx
Outdated
| const Loading: React.FC<Props> = ({ text, ...rest }: Props) => ( | ||
| <div {...rest}> | ||
| <div className="nhsuk-loader__container"> | ||
| <p className="nhsuk-heading-l">{text}</p> |
There was a problem hiding this comment.
Would we want to render this element if no text prop is supplied?
There was a problem hiding this comment.
I was thinking of just using a default value of "Loading..." if text isn't supplied, what do you think?
There was a problem hiding this comment.
I meant more of an empty string - so if someone wanted to disable the text entirely it wouldn't render an empty <p> tag (which I think can fail some Jest-axe tests but that might only be headers)
There was a problem hiding this comment.
Ok, i'll have it check for the text being non-empty 👍
On that note, do you think it's worth adding a withoutText boolean prop for removing the text entirely? Seems like it'd be more intuitive than supplying an empty string to get around the default behaviour
There was a problem hiding this comment.
Or maybe allow for similar handling to errors in some components - i.e. allow for the type of text to be string | false and then check for false, and otherwise display any text passed in
src/components/loading/_Loading.scss
Outdated
|
|
||
| .nhsuk-loader { | ||
| pointer-events: none; | ||
| width: 2.5em; |
There was a problem hiding this comment.
Should we be specifying the width on the .nhsuk-loader itself? The only way a user would be able to change the size of the spinner would be to directly override the .nhsuk-loader class. Would it be better to ensure that this element takes up 100% of it's container, then set the sizing on the container itself?
2 participants
PR to add a loading component created as part of Cervical Screening platform.