Overlay & Styled-components 4.4.0 #20
Conversation
|
@jonashaefele can you have a look at my |
luckylwk
changed the title
There was a problem hiding this comment.
This is great.
Left a comment re: Overlay/Navigation organisation. Otherwise great. A sentence or two where it says TODO and we're all set.
Just FYI, I branched off last week to port HDS to docz 2.0. It's a manual job, lots of imports need to change, etc, so will have to port these new components over when I'm done. Just help me check which bits were added, so we don't forget any changes. I'm pretty much done, just tried to make it prettier, but it's not that pretty to look at yet.
| NavCenter.displayName = 'NavCenter'; | ||
|
|
||
| // --------------------------- |
There was a problem hiding this comment.
guess these are a thing you like to separate sibling components in one file?
|
|
||
| import Color from 'color'; | ||
|
|
||
| // --------------------------- |
There was a problem hiding this comment.
Trying to figure out when to add them and when not to?
|
|
||
| // --------------------------- | ||
|
|
||
| const NavOverlay = styled(Overlay)` |
There was a problem hiding this comment.
Only thing that annoys me a bit is that part of Nav is now in this new Overlay file... I like the re-usable Overlay, but it's hard to find all the bits that belong to Navigation now.
I'd probably define NavOverlay where it was before.
As in keep it in Navigation.js, you can still import { Overlay } from ../Overlay and then define NavOverlay just as you did here...
| z-index: ${({ theme }) => theme.zIndices.overlay - 1}; | ||
|
|
||
| background-color: ${({ theme }) => | ||
| Color(theme.colors.whiteout.lightest) |
There was a problem hiding this comment.
This makes overlays always white. Is there a case to make this a prop? bg='primary.lighter' or do we want them to be white always? (Could actually be nice)
| ExpandableBody, | ||
| // ExpandableContext | ||
| } from '../Expandable'; | ||
| import { HeartLoader } from 'components/Loader'; |
There was a problem hiding this comment.
for docz 2.2:
import { HeartLoader } from '../Loader';
There was a problem hiding this comment.
think that's the only realy change you need to make it work with master/docz2.2
then just run yarn run build && yarn run prettier:all again and the rest of the conflicts should be resolved.
✅ DONE:
Because of issues that nextjs has in development (not in production!) with the server-side-rendering. See:
⏹ TODO:
v1.0.1