Skip to content

Improve top bar and main graph stats UI#6245

Open
sanne-san wants to merge 12 commits intomasterfrom
sanne-dashboard-top-bar
Open

Improve top bar and main graph stats UI#6245
sanne-san wants to merge 12 commits intomasterfrom
sanne-dashboard-top-bar

Conversation

@sanne-san
Copy link
Copy Markdown
Contributor

@sanne-san sanne-san commented Apr 13, 2026

Changes

Top bar

  • Add consistent styling and spacing for all buttons in the top bar
  • Move period arrows into the period picker visually instead of displaying them as separate buttons
  • Add selected state to buttons; when dropdown is open, the trigger background is light gray. This is also true when comparison is enabled.
  • Change filter button icon from magnifying glass to a filter icon
  • Change period picker button icons from chevron on the right to calendar icons on the left
  • Make top bar scroll horizontally on mobile

New options menu (⋮)

  • Move interval picker, export button, imported data toggle and notices out of the graph and into a new options menu in the top bar
  • Hide menu in realtime mode
  • Render button immediately as a non-interactive placeholder on page load to prevent layout shift
  • Change interval picker dropdown to a segmented control
  • Change the imported data toggle to an actual toggle button
  • Change notice icon + tooltip to an inline notice component
  • The i keyboard shortcut now cycles through graph intervals rather than opening the dropdown menu – also when the options menu is closed

Top stats

  • Change selected metric background highlight to a subtle background instead of a coloured underline on the label
  • Refine stat labels and values typography and colors
  • Ensure change arrows and percentages are shown also when in comparison mode (and removed from tooltip)
  • Show "until HH:MM" when "Today" is the comparison date range to indicate a partial day

Code organisation

  • Introduce a shared React context (DashboardOptionsProvider) to pass graph state (intervals, import settings) up to the top bar without prop drilling
  • Move custom icons to a shared icons file
  • Extract segmented control, toggle button and notice as reusable components
  • Old standalone StatsExport, WithImportedSwitch, and NoticesIcon components removed; functionality consolidated into the options menu

Before and after:
https://github.com/user-attachments/assets/ec519314-629f-4034-87d9-9cded458f646

New options menu:
CleanShot 2026-04-14 at 11 06 32@2x

Tests

  • Automated tests have been added

Changelog

  • Entry has been added to changelog

Documentation

  • Docs have been updated

Dark mode

  • The UI has been tested both in dark and light mode

Top bar
- Add consistent styling and spacing for all buttons in the top bar
- Move period arrows into the period picker visually instead of displaying them as separate buttons
- Add selected state to buttons; when dropdown is open, the trigger background is light gray. This is also true when comparison is enabled.
- Change filter button icon from magnifying glass to a filter icon
- Change period picker button icons from chevron on the right to calendar icons on the left
- Make top bar scroll horizontally on mobile instead of clipping

New options menu (⋮)
- Move interval picker, export button, imported data toggle and notices out of the graph and into a new options menu in the top bar
- Hide menu in realtime mode
- Render button immediately as a non-interactive placeholder on page load to prevent layout shift
- Change interval picker dropdown to a segmented control
- Change the imported data toggle to an actual toggle button
- Change notice icon + tooltip to an inline notice component

Top stats
- Change selected metric background highlight to a subtle background instead of a coloured underline on the label
- Refine stat labels and values typography and colors
- Ensure change arrows and percentages are shown also when in comparison mode (and removed from tooltip)
- Show "until HH:MM" when "Today" is the comparison date range to indicate a partial day

Code organisation
- Introduce a shared React context (DashboardOptionsProvider) to pass graph state (intervals, import settings) up to the top bar without prop drilling
- Move custom icons to a shared icons file
- Extract segmented control, toggle button and notice as reusable components
- Old standalone StatsExport, WithImportedSwitch, and NoticesIcon components removed; functionality consolidated into the options menu
@sanne-san sanne-san force-pushed the sanne-dashboard-top-bar branch from d0eeb10 to 269eac4 Compare April 13, 2026 16:31
@sanne-san sanne-san force-pushed the sanne-dashboard-top-bar branch from 07c0dc1 to f334e4d Compare April 14, 2026 09:05
@sanne-san sanne-san marked this pull request as ready for review April 14, 2026 09:12
@sanne-san sanne-san added preview and removed preview labels Apr 14, 2026
@github-actions
Copy link
Copy Markdown

Preview environment👷🏼‍♀️🏗️
PR-6245

@sanne-san sanne-san requested a review from a team April 14, 2026 09:32
@ukutaht
Copy link
Copy Markdown
Contributor

ukutaht commented Apr 14, 2026

Tested on preview. Feels great to me overall. A couple things I noticed while testing:

1. Date picker has uneven padding when arrows not shown

Screenshot 2026-04-14 at 14 07 21

The padding looks good with arrows.

2. Date picker dropdown has some weirdness

I cannot close the dropdown by clicking outside of it just above the datepicker: https://cap.so/s/8dg13mgn12zk6bb

Also I cannot replicate this for some reason but once I did manage to get it into this state where the focus ring shows and does not look great

Screenshot 2026-04-14 at 14 08 54

@ukutaht
Copy link
Copy Markdown
Contributor

ukutaht commented Apr 14, 2026

Don't want to get too much into small matters of taste, but I couldn't help but notice that in the new variant the 'current visitors' text looks noticeably heavier than the site domain:

Screenshot 2026-04-14 at 14 59 25

As opposed to the current version where the domain is heavier than current visitors:

Screenshot 2026-04-14 at 15 00 25

The new version looks a bit strange to me but I can't put my finger on why or justify it too much :) did you choose it deliberately and prefer it this way?

- Refactor site switcher to use a static component when not logged in
- Add test for static site switcher
- Fix padding on period picker when arrows are hidden
- Fix focus-visible ring on period picker
- Update typography for site switcher and current visitors
@sanne-san sanne-san force-pushed the sanne-dashboard-top-bar branch from ac3223d to 7575f95 Compare April 16, 2026 11:30
@sanne-san
Copy link
Copy Markdown
Contributor Author

@ukutaht good catches, fixed them! RE typography of the site switcher and current visitors: it was a choice between consistency and hierarchy, and I opted for more consistency. I was hesitating as well though so I reverted it to the original. I think because the site domain isn't capitalised and the current visitors starts with a number, the latter feels heavier somehow.

@sanne-san sanne-san requested review from RobertJoonas and apata April 20, 2026 15:23
Comment on lines -264 to -265
height: SEE_MORE_WIDTH_PX,
width: SEE_MORE_WIDTH_PX,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nitpick, non-blocking: The reason size was given with style is for the actual size and the size used in calculations not to be out of sync. Now, the sync between size-8 and SEE_MORE_WIDTH_PX has to be managed manually.

{showCurrentVisitors && <CurrentVisitors />}
</div>
<div className="flex w-full">
<div className="flex min-w-0 flex-1">
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

issue, non-blocking: Now this section can shrink to nothing. Image

}: {
children: React.ReactNode
}) {
const [options, setOptions] = useState<DashboardOptionsContextValue | null>(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

issue, non-blocking: This is duplicative state (between visitor-graph and this component), which causes us to need syncing code. I think a better split would be to have

GraphIntervalContext - solely responsible for calculating available intervals and the current one, and providing the means to change the current one

ImportsIncludedContext - same as above, just for imports, may itself depend on GraphIntervalContext

and to use the exposed context state in the components where they're needed, when fetching the graph and in the new triple dot menu.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Should ImportsIncludedContext own its own query, or is a push from visitor-graph fine since the imports state comes from the top stats fetch?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Valid question, I think that needs to be pushed.

)
})

describe(`${getPartialDayTimeRange.name}`, () => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

suggestion, non-blocking: Let's also test that it handles timezones in the date string (I've not actually seen 2024-01-13T00:00:00, always something like

        "date_range": [
            "2026-01-19T00:00:00+00:00",
            "2026-04-19T23:59:59+00:00"
        ],

return { topStats, meta, from, to, comparingFrom, comparingTo, timeRange }
}

// Returns "until HH:MM" when the date range is a partial day (period=day for
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

suggestion, non-blocking: If the comment about the function is exactly above the function, IDEs include it when hovering at a call site.

/** Does something complex, explanation */ 
const f = () => {
//  ..
}

{formatDateRange(site, data.comparingFrom, data.comparingTo)}
<p className="text-xs text-gray-500 dark:text-gray-400 font-medium">
{data.timeRange
? `${formatDayShort(parseUTCDate(data.comparingFrom))}, ${data.timeRange}`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

issue, blocking: This works for Today vs Previous period, but it is incorrect when comparing Today vs Custom Period (pick yesterday from comparison calendar). The comparison query is made for the whole day, so we'd need to calculate and show comparisonTimeRange here.


expect(arrowElement).toHaveTextContent('↑ 1%')
expect(arrowElement.children[0]).toHaveClass('text-green-500')
expect(arrowElement.children[0]).toHaveAttribute('data-direction', 'up')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

question, non-blocking: Why did we get rid of the color asserts? I think they matter because bounce rate down being rendered as green is important. Up/down direction is already tested with toHaveTextContent('↑ 1%'). (The arrow icons are mocked with a particular unicode character)

className={classNames(
popover.toggleButton.classNames.rounded,
'gap-x-1.5 px-2.5 font-medium text-gray-700 dark:text-gray-100',
'!pl-0'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

question, non-blocking: why padding left with !important? I don't see padding left previously declared in popover.toggleButton.classNames.rounded, just on the prev line.

'hover:bg-gray-100 dark:hover:bg-gray-800'
popover.toggleButton.classNames.rounded,
popover.toggleButton.classNames.ghost,
'!pl-1.5'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

question, non-blocking: Does the switcher not need same left & right padding? E.g. !px-1.5

Comment on lines 627 to +643
@@ -627,10 +640,7 @@ test.describe('location filtering tests', () => {

await applyFilterButton(page).click()

await page
.getByRole('button', { name: 'See 1 more filter and actions' })
.click()

await page.getByRole('button', { name: /See.*more/ }).click()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

issue, non-blocking: These tests are running on a constant viewport (1024px). "See N more" depends on that, filter pills style, and other top bar items.

Because the UI changed, the conditions to wrap to to "See N more" changed, so I can see why you added another filter. However, the comment is misleading and there's some missing asserts now.

In these tests, I think we should definitively check which filters of the added filters are visible without clicking "See more", that "See more" has the correct label, and that all the rest are visible after clicking "See more".

@apata
Copy link
Copy Markdown
Contributor

apata commented Apr 20, 2026

Great work! Pretty major changes at first glance, but opening the app it feels very familiar and natural, at least to me. With the top stats enhanced time labels issue solved (#6245 (comment)), it's ready to go as far as I'm concerned.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants