feat: allow new insight type choice from sidebar #14759
Conversation
| @@ -299,31 +280,9 @@ export function NewInsightButton({ dataAttr }: NewInsightButtonProps): JSX.Eleme | |||
| placement: 'bottom-end', | |||
| className: 'new-insight-overlay', | |||
There was a problem hiding this comment.
Wow, this dropdown is really w i d e! I haven't notice before, because, well, I've always used the plus in the sidebar. But to ship this, let's constraint the width of .new-insight-overlay to something like 21rem (this number seems to result in all options having roughly equal-size two lines of description)
|
Ah, GitHub ate my review summary… Basically: |
Co-authored-by: Michael Matloka <dev@twixes.com>
I thought "Oh yeah, you can click the plus" I never do, which made me wonder if other people do. Last 7 days only 7% of clicks on the insight and plus area were on the plus
Hmmm, numbers but maybe not insight :rofl |
|
|
| &[data-attr='sidebar-new-insights'] { | ||
| max-width: 21rem; | ||
| } |
There was a problem hiding this comment.
This style would also be useful on the "New insight" popover on the insights list page. Can we put it on .new-insight-overlay instead of this SideBar override?
There was a problem hiding this comment.
Hmmm, interesting, that already has an override that's being ignored
*rolls up sleeves
There was a problem hiding this comment.
How about passing optionally passing max width to popover....
| @@ -120,7 +123,7 @@ export const Popover = React.forwardRef<HTMLDivElement, PopoverProps>(function P | |||
| apply({ availableWidth, availableHeight, rects, elements: { floating } }) { | |||
| Object.assign(floating.style, { | |||
| maxHeight: `${availableHeight}px`, | |||
| maxWidth: `${availableWidth}px`, | |||
There was a problem hiding this comment.
So, the reason this is potentially dangerous is now we're not taking availableWidth into account – and we always should be. But if we just set width instead of max-width in .new-insight-overlay, this'd work well – that width would be used most of the time, but max-width would automatically take precedence when needed. (Plus, we wouldn't need to extend the Popover API)
This reverts commit 415a751.
The tooltip basically functions as a on-hover label here, so using the same text as on the saved insights page, where it says "New insight".
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
* master: (101 commits) feat: feedback tab - improve the UI and switch to HogQL (#14777) feat(experiments): cache experiment results (#14742) fix(empty-state): properly fix query timeout state (#14793) chore(feature-flag): release json payload flag (#14775) feat(hogql): make sure joins work with properties (#14790) test(frontend): Tune visual regression test failure threshold (#14766) feat(hogql): count distinct and count star (#14786) feat(perf): only load experiment results once (#14772) feat: allow new insight type choice from sidebar (#14759) fix(persons): hide persons in the future (#14308) feat: allow query cards in saved insights grid (#14784) fix(lemon-ui): Align padding for small/large buttons with icons (#14750) chore(deps): Upgrade TypeScript from 4.8 to 4.9 in the frontend (#14609) feat: rename insight query tab to json tab (#14781) feat: use same top heading for insight card and exported insight (#14780) feat: define new insight urls in one place (#14778) fix(hogql): person properties in a funnel breakdown (#14765) fix: hide query based insights in recently viewed list (#14774) feat: improve query summaries (#14768) feat(hogql): add sampling support (#14733) ...
|
Removing it in #14876 |
Problem
If you want to create a particular kind of new insight it can take over two clicks. Nobody has time for that
Changes
Now it takes two clicks!
How did you test this code?
👀 locally