Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(3000): Welcome panel #18842

Merged
merged 73 commits into from
Dec 5, 2023
Merged

feat(3000): Welcome panel #18842

merged 73 commits into from
Dec 5, 2023

Conversation

benjackwhite
Copy link
Contributor

@benjackwhite benjackwhite commented Nov 23, 2023

Problem

Some viewports can end up with really small banners which get very cramped.
Also we wanted a welcome banner. Merged into the same PR as I was working with both together at first, before deciding against the banner in the welcome page

Changes

For reviewers: You can access it always simply with http://localhost:8000/home#panel=welcome

  • Use a container query to change the layout optimising for space.
  • Added a Welcome panel with its own flag that we can setup for existing posthog users only.
  • Not married to the copy or list - really happy to have suggestions there.
  • Adds stories for the side panel

Screenshot 2023-11-29 at 16 23 14

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

How did you test this code?

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

8 snapshot changes in total. 0 added, 8 modified, 0 deleted:

  • chromium: 0 added, 8 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

Copy link
Contributor

github-actions bot commented Nov 23, 2023

Size Change: +76 B (0%)

Total Size: 1.83 MB

ℹ️ View Unchanged
Filename Size Change
frontend/dist/toolbar.js 1.83 MB +76 B (0%)

compressed-size-action

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

9 snapshot changes in total. 0 added, 9 modified, 0 deleted:

  • chromium: 0 added, 9 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@benjackwhite benjackwhite changed the title feat: Improve banner layout with container queries feat: Welcome panel and improved banner layout with container queries Nov 23, 2023
@benjackwhite benjackwhite requested a review from a team November 23, 2023 15:20
@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

  • chromium: 0 added, 2 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@benjackwhite benjackwhite marked this pull request as ready for review November 23, 2023 16:16
@corywatilo corywatilo self-assigned this Nov 23, 2023
Copy link
Member

@Twixes Twixes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very cool. As for the copy and design, gonna give @corywatilo a chance to poke at this

@corywatilo corywatilo requested a review from a team December 1, 2023 23:15
@corywatilo
Copy link
Contributor

corywatilo commented Dec 1, 2023

FYI I changed the tab to say "What's new?". We can use it for specific updates when we want to in the future. (Different than changelog.)

Todos

  • Get button to open in a new window (currently opens in sidebar)
  • Fix above-mentioned button where I inlined display: inline-flex which the linter is not too happy about
  • Update button CTA to: /blog/posthog-as-a-dev-tool (to match this post)
  • Add in @lottiecoxon art (when it's ready) - not a blocker to merging

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

  • chromium: 0 added, 2 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@Twixes Twixes changed the title feat: Welcome panel and improved banner layout with container queries feat(3000): Welcome panel Dec 5, 2023
@Twixes Twixes force-pushed the feat/welcome-to-3000 branch from e99b536 to a9b9675 Compare December 5, 2023 16:38
@PostHog PostHog deleted a comment from posthog-bot Dec 5, 2023
@Twixes Twixes force-pushed the feat/welcome-to-3000 branch from 438a782 to becee00 Compare December 5, 2023 16:54
@PostHog PostHog deleted a comment from posthog-bot Dec 5, 2023
@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

4 snapshot changes in total. 0 added, 4 modified, 0 deleted:

  • chromium: 0 added, 4 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

Copy link
Member

@Twixes Twixes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new layout is great! I addressed the to-dos (except Lottie's art) and made some extra tweaks, here's what this looks like now:
Screenshot 2023-12-05 at 17 54 44
This should be good to merge. I'm not sure if good to release, because I couldn't see the logic to set a person property on side panel close, but that's not PR-blocking.

Comment on lines +106 to +108
We're past zero to one.
<br />
In this new version of PostHog, we're going from one… to&nbsp;3000.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most style guides have a rule for spelling the numbers out when they're under ten (sometimes even under a hundred), because "zero to one" is much more legible than "0 to 1" in a non-technical context. I think the startup-y reference is also clearer with "zero to one" rather than "0 to 1", because the former is what the book is called.
This is a bit tricky because then we're introducing 3000, which is a large number. I added an ellipsis to make the context shift explicit, maybe that helps?

</Card>
<Card className="flex flex-col">
<div className="flex-1">
<Title>Command bar</Title>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "command palette" is the old name… the new iteration is called the "command bar"

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

4 snapshot changes in total. 0 added, 4 modified, 0 deleted:

  • chromium: 0 added, 4 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@Twixes Twixes merged commit 62e216b into master Dec 5, 2023
@Twixes Twixes deleted the feat/welcome-to-3000 branch December 5, 2023 19:42
Twixes added a commit that referenced this pull request Dec 6, 2023
* Improve banner layout with container queries

* Update UI snapshots for `chromium` (1)

* Fix

* Update UI snapshots for `chromium` (1)

* Fix

* Fixes

* Fix up closing

* Started adding stories

* Fixed up when it gets shown

* Fixes

* Update UI snapshots for `chromium` (1)

* Fixes

* fix

* Update UI snapshots for `chromium` (1)

* Fix

* PR feedback

* Added event for the flag

* updates

* Bunch of fixes

* Update UI snapshots for `chromium` (1)

* Update UI snapshots for `chromium` (2)

* Fixes

* Fix lemon banner small size and add sidepanel stories

* Fix

* Update UI snapshots for `chromium` (1)

* Update frontend/src/layout/navigation-3000/sidepanel/panels/SidePanelWelcome.tsx

Co-authored-by: Thomas Obermüller <[email protected]>

* Fix

* Update UI snapshots for `chromium` (1)

* Update UI snapshots for `chromium` (2)

* Fix

* Rework the welcome copy and tune styles

* Remove a "the"

* Update UI snapshots for `chromium` (1)

* Update UI snapshots for `chromium` (1)

* Update UI snapshots for `chromium` (2)

* Update UI snapshots for `chromium` (1)

* Update UI snapshots for `chromium` (2)

* Update UI snapshots for `chromium` (2)

* updated design + content

* transparent close button

* dark mode images

* finished dark mode screenshots

* polish

* updated welcome icon, label

* updated welcome title to match tab

* tweaked spacing

* Update UI snapshots for `chromium` (1)

* Limit opening posthog.com links in side panel to docs

* Clean up the panel

* Tune default and min width of side panel

* Tweak some text

* The palette is no more

* Update UI snapshots for `chromium` (1)

* Update UI snapshots for `chromium` (1)

* Update UI snapshots for `chromium` (1)

* Constrain "Read the blog post" width

* Update UI snapshots for `chromium` (1)

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Cory Watilo <[email protected]>
Co-authored-by: Thomas Obermüller <[email protected]>
Co-authored-by: Michael Matloka <[email protected]>
@joethreepwood
Copy link
Contributor

joethreepwood commented Dec 6, 2023

Not to rain on everyone's parade, but I quite strongly that this new welcome panel isn't readable and needs to be rolled back. The announcement looks great viewed in isolation, but when viewed in a sidebar alongside the rest of PostHog...

Screenshot 2023-12-06 at 10 12 23

There's so much text on the page, no discernable pattern for a grid to understand it across the left and right hand sides of the page, and the level of contrast between the images and the actual text make it hard to know which is which. Some sections are zoomed in, some aren't. The images on the right are clipped off, which makes me feel like I'm not seeing the full image.

Even after looking at it for a few minutes and trying to read through item-by-item, I still find it hard to follow and really difficult to look at. And that's when I'm prepared to see PostHog 3000 - actual users will be even more overwhelmed because they won't expect this.

We could try to address the hierarchy by making the welcome message boxes order and highlight better, removing images - but my direct feedback is that we'd be best off reverting to the previous version and laying this information out as simply as possible.

EDIT: additional feedback here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants