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.

Sign up for GitHub

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

Jump to bottom

feat: UI tweaks to Web Analytics #20359

Merged
merged 3 commits into from
Feb 15, 2024
Merged

feat: UI tweaks to Web Analytics #20359

merged 3 commits into from
Feb 15, 2024

Conversation

benjackwhite
Copy link
Contributor

@benjackwhite benjackwhite commented Feb 15, 2024 •
edited
Loading

Problem

Noticed some things that looked off so I thought I'd take 5 mins to see if it could be spruced up :)

Changes

Before After
Screenshot 2024-02-15 at 11 36 57 Screenshot 2024-02-15 at 11 35 57
Screenshot 2024-02-15 at 11 37 16 Screenshot 2024-02-15 at 11 36 32
Screenshot 2024-02-15 at 11 37 24 Screenshot 2024-02-15 at 11 38 11
Screenshot 2024-02-15 at 11 42 04 Screenshot 2024-02-15 at 11 41 32
  • Added skeleton class for top level overview tabs so it doesnt jump in
  • Added border and background to match other items
  • Removed the complicated distribution class in favour of some simple flexing for mobile
  • Ensure some other graphs fill their height which ensures two adjacent graphs don't have different heights.
  • Moves date filter to first position so it doesnt jump around when setting other filters

👉 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

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

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

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, 0 modified, 0 deleted
  • webkit: 0 added, 1 modified, 0 deleted (diff for shard 2)

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 15, 2024 •
edited
Loading

Size Change: 0 B

Total Size: 2.22 MB

ℹ️ View Unchanged
Filename Size
frontend/dist/toolbar.js 2.22 MB

compressed-size-action

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

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

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

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@benjackwhite benjackwhite marked this pull request as ready for review February 15, 2024 11:42
@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 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

robbie-c
robbie-c requested changes Feb 15, 2024
View reviewed changes
frontend/src/queries/nodes/WebOverview/WebOverview.tsx
const webOverviewQueryResponse = response as WebOverviewQueryResponse | undefined

const samplingRate = webOverviewQueryResponse?.samplingRate

return (
<>
<EvenlyDistributedRows className="w-full gap-x-2 gap-y-8" minWidthRems={8}>
Copy link
Member

Choose a reason for hiding this comment

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

I like the background but I'd still do this with EvenlyDistributedRows, so that if there's space for 4 items they still get split 3-2 rather than 4-1

Screenshot 2024-02-15 at 11 47 31

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 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, 0 modified, 0 deleted
  • webkit: 0 added, 1 modified, 0 deleted (diff for shard 2)

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, 0 modified, 0 deleted
  • webkit: 0 added, 1 modified, 0 deleted (diff for shard 2)

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@robbie-c robbie-c force-pushed the fix/web-analytics-ui branch from c3c0351 to 096f354 Compare February 15, 2024 16:31
@robbie-c robbie-c force-pushed the fix/web-analytics-ui branch from 096f354 to f60d092 Compare February 15, 2024 16:31
@robbie-c robbie-c self-requested a review February 15, 2024 16:33
@robbie-c robbie-c enabled auto-merge (squash) February 15, 2024 16:33
@robbie-c robbie-c merged commit 5e1a5d4 into master Feb 15, 2024
79 checks passed
@robbie-c robbie-c deleted the fix/web-analytics-ui branch February 15, 2024 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robbie-c robbie-c robbie-c approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@benjackwhite @posthog-bot @robbie-c