-
Notifications
You must be signed in to change notification settings - Fork 32
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.
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
Replace uses of style prop #3345
Conversation
81c5c00
to
cd17194
Compare
cd17194
to
e1845e2
Compare
@@ -14,6 +14,7 @@ export const StyledSidebar = styled.aside<StyledSidebarProps>` | |||
flex-direction: column; | |||
position: relative; | |||
z-index: ${zIndex('sidebar', 0)}; | |||
flex-shrink: 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing the redundant intermediary <div>
in the stories revealed this would be shrunk slightly when Sidebar
is directly in SidebarWrapper
, so this was added to fix that.
@@ -89,18 +95,8 @@ export const Default: ComponentStory<typeof Sidebar> = (props) => { | |||
|
|||
return ( | |||
<SidebarWrapper> | |||
<div style={{ maxHeight: '30rem' }}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These intermediary div
s were redundant as StyledSidebar
has max-height
applied to it already. Removing them did reveal a small problem with the StyledSidebar
styles (see comment left there).
it('is is rendered below the mouse pointer', () => { | ||
// jest-styled-components doesn't work with css props: | ||
// https://github.com/styled-components/jest-styled-components/issues/250 | ||
it.skip('is is rendered below the mouse pointer', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jest-styled-components doesn't understand css props unfortunately:
ad47a51
to
5b0ab49
Compare
offset: string | ||
}) => ( | ||
<span | ||
className="rn_text-s" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was an old CSS framework class here!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
5b0ab49
to
8c35df1
Compare
This replaces all uses of style props in components with preferred Styled Components patterns.
This replaces all uses of style props in stories with alternative Styled Components patterns.
This stops the width of `Sidebar` being shrunk when it's placed directly in `SidebarWrapper` (causing it to be less than 60px wide).
8c35df1
to
1d84a3a
Compare
Kudos, SonarCloud Quality Gate passed! |
Related issue
Resolves #2600
Overview
This replaces all uses of style props with patterns from Styled Components.
Link to preview
https://5e25c277526d380020b5e418-qvmgkmtfwt.chromatic.com/
Reason
Work carried out
Developer notes
Stories were updated as well for consistency and to make sure there are no more hits for
style=
in code.