report(flow): add help dialog to explain flows #13159
Conversation
flow-report/src/help-dialog.tsx
Outdated
| const strings = useUIStrings(); | ||
|
|
||
| return ( | ||
| <div className={classNames('HelpDialog', {'HelpDialog--hidden': !visible})}> |
There was a problem hiding this comment.
Instead of adding a --hidden style, WDYT about removing the node entirely? This way you don't need the visible param and it won't be rendered in the DOM for most use cases.
There was a problem hiding this comment.
sure we can go with conditional rendering here. my thoughts on this topic in general are to default to display: none if the content is not exceedingly expensive to keep around in the DOM because you get DOM-based state preservation for free, and it avoids really thorny bugs in many SSR hydration implementations. in this case though, we're definitely not SSR'ing and there's no state to keep, so I'm relatively indifferent.
| @@ -92,6 +96,16 @@ export const Topbar: FunctionComponent<{onMenuClick: JSX.MouseEventHandler<HTMLB | |||
| onClick={() => saveHtml(flowResult, dom)} | |||
| label="Button that saves the report as HTML" | |||
| >{strings.save}</TopbarButton> | |||
| <div style={{flexGrow: 1}} /> | |||
| <TopbarButton | |||
There was a problem hiding this comment.
Mocks show this button with blue text, up to you if you want to change it or keep it consistent with the other topbar buttons.
There was a problem hiding this comment.
I figured we'd keep it consistent, but happy to discuss tomorrow with jiwoong if he feels strongly about it
There was a problem hiding this comment.
Feel free to merge without the blue text since this is blocking your next PR. We can circle back to this after the UX sync.
Summary
Implements the help dialog from the mocks. Waiting on #13109 to resolve the topbar button layout before updating the button here, but the contents should be unaffected.
Related Issues/PRs
ref #11313