feat: aside header top alert #118
Conversation
|
Preview is ready. |
| return ( | ||
| <AsideHeaderContextProvider value={asideHeaderContextValue}> | ||
| <AsideHeaderInnerContextProvider value={asideHeaderInnerContextValue}> | ||
| <div className={b({compact}, className)}> | ||
| {/* Top Panel */} | ||
| <TopPanel ref={topRef} /> |
There was a problem hiding this comment.
Can we not render the component if topAlert is not set?
goshander
force-pushed
the
feat-aside-header-top-block
branch
from
88e7cc1 to
caeb84e
Compare
|
@DarkGenius i think need waiting this PR #140, right? |
Yes, it’s possible that after this PR is merged, changes will be required in your PR |
goshander
force-pushed
the
feat-aside-header-top-block
branch
from
afbea1c to
d2494b8
Compare
src/components/AsideHeader/types.tsx
Outdated
| import {AsideHeaderContextType} from './AsideHeaderContext'; | ||
|
|
||
| export interface LayoutProps { | ||
| compact: boolean; | ||
| className?: string; | ||
| topRef?: React.RefObject<HTMLDivElement>; | ||
| topAlert?: AsideHeaderTopAlertProps; | ||
| updateTopSize?: () => void; |
There was a problem hiding this comment.
It's better to rename it to onCloseTopAlert. Because arbitrary code can be passed to this property.
goshander
force-pushed
the
feat-aside-header-top-block
branch
from
2a4c5e1 to
18b751e
Compare
| @@ -80,7 +80,7 @@ body { | |||
| &__settings-panel, | |||
| &__search-panel { | |||
| width: 300px; | |||
| height: 100%; | |||
| height: calc(100% - 40px); | |||
There was a problem hiding this comment.
fix drawer padding at 20px for remove unexpected scroll
| } | ||
| return () => { | ||
| window.removeEventListener('resize', updateTopSizeDebounce); | ||
| setAsideTopPanelHeight(0); |
There was a problem hiding this comment.
set top panel size to zero for correct destroy component behaviour with iframe switching at storybook
fixed |
| @@ -2,6 +2,10 @@ | |||
|
|
|||
| $block: '.#{variables.$ns}aside-header'; | |||
|
|
|||
| :root { | |||
| --gn-aside-top-panel-height: 0px; | |||
There was a problem hiding this comment.
Why is it not in the .g-root?
There was a problem hiding this comment.
Then we can not change this value from document object
There was a problem hiding this comment.
We can use the correct selector
There was a problem hiding this comment.
We can use the correct selector
Ok, changed to getElementsByClassName selector
| @@ -1,4 +1,4 @@ | |||
| import React, {FC, useState} from 'react'; | |||
| import React, {FC} from 'react'; | |||
There was a problem hiding this comment.
Maybe then we’ll remove FC import too?
I want to implement something like this
