feat: Add Toolbar component to teams-components#311
feat: Add Toolbar component to teams-components#311ling1726 wants to merge 4 commits intomicrosoft:mainfrom
Conversation
| }, | ||
| ref | ||
| ); | ||
| useDividerStyles_unstable(state); |
There was a problem hiding this comment.
I would double check if we really want to reuse Divider there
| export * from './Toolbar'; | ||
| export * from './ToolbarDivider'; | ||
| export * from './ToolbarButton'; | ||
| export * from './ToolbarToggleButton'; | ||
| export * from './ToolbarMenuButton'; |
There was a problem hiding this comment.
This is too wild, let's be explicit
| private _treeWalker: TreeWalker; | ||
| private _filter: ElementFilter; |
There was a problem hiding this comment.
| private _treeWalker: TreeWalker; | |
| private _filter: ElementFilter; | |
| #treeWalker: TreeWalker; | |
| #filter: ElementFilter; |
let's be cool
| const validateToolbarItems = (root: HTMLElement) => { | ||
| const children = root.children; | ||
| for (const child of children) { | ||
| // TODO is this even possible? | ||
| if (!isHTMLElement(child)) { | ||
| continue; | ||
| } | ||
|
|
||
| if ( | ||
| !isAllowedToolbarItem(child) && | ||
| !isPortalSpan(child) && | ||
| !isTabsterDummy(child) | ||
| ) { | ||
| throw new Error( | ||
| '@fluentui-contrib/teams-components::Toolbar::Use Toolbar components from @fluentui-contrib/teams-components package only' | ||
| ); | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
We will definitely need a nicer validator
| const treeWalker = new HTMLElementWalker(elRef.current, (el) => { | ||
| if (isAllowedToolbarItem(el) || el === treeWalker.root) { | ||
| return NodeFilter.FILTER_ACCEPT; | ||
| } | ||
|
|
||
| return NodeFilter.FILTER_REJECT; | ||
| }); |
There was a problem hiding this comment.
TreeWalker is nice, but why? element.children with a filter will do the same
| if (prev && el.dataset.appearance !== 'transparent') { | ||
| prev.style.marginInlineStart = tokens.spacingHorizontalS; |
There was a problem hiding this comment.
Have to say, that this should be somehow declarative as it will go will quite quickly.
Dumb idea:
const STYLE_SCHEMAS = {
kind: 'ToolbarButton',
prevKind: 'ToolbarDivider',
nextKind: null,
meta: { appearance },
style: { marginInlineStart: tokens.spacingHorizontalS },
}
function applyStylesToElement() {}
There was a problem hiding this comment.
As discussed offline - will try out registerProperty API without inheritable
There was a problem hiding this comment.
we could try leverage default react context or other means to make sure the cost of property registration only happens with usage
|
|
||
| export const useItemRegistration = (metadata: ToolbarItemMetadata) => { | ||
| const { registerItem } = React.useContext(ToolbarItemRegistrationContext); | ||
| const styles = useItemRegistrationStyles(); |
There was a problem hiding this comment.
not sure about mixing styles there
| let propertyRegisterComplete = false; | ||
|
|
||
| export const registerCustomProperties = (win: typeof globalThis) => { | ||
| if (propertyRegisterComplete) { |
There was a problem hiding this comment.
Should be done per window?
| const styles = useItemRegistrationStyles(); | ||
| const ref = React.useCallback((el: HTMLElement) => { | ||
| if (el) { | ||
| registerItem(el, metadata); |
There was a problem hiding this comment.
We don't react on meta updates. Considering that it's an object, might be tricky
2 participants
No description provided.