Improve sidebar#484
Draft
okt-limonikas wants to merge 35 commits into
Draft
Conversation
okt-limonikas
force-pushed
the
sidebar-refactor
branch
4 times, most recently
from
ea0f55d to
0f8879e
Compare
okt-limonikas
force-pushed
the
sidebar-refactor
branch
4 times, most recently
from
aaf7f53 to
1e4d7dd
Compare
okt-limonikas
force-pushed
the
sidebar-refactor
branch
6 times, most recently
from
2644ed8 to
eed7621
Compare
okt-limonikas
force-pushed
the
sidebar-refactor
branch
from
eed7621 to
b2e2235
Compare
okt-limonikas
force-pushed
the
sidebar-refactor
branch
2 times, most recently
from
a29504f to
44ffa39
Compare
okt-limonikas
force-pushed
the
sidebar-refactor
branch
2 times, most recently
from
1b9cade to
4bf6ffe
Compare
okt-limonikas
force-pushed
the
sidebar-refactor
branch
4 times, most recently
from
ecd0a76 to
677e7b4
Compare
Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
Add sidebar primitive components for construction of sidebar items. This will allow different features to build custom sidebar items from primitives with custom logic like displaying loading indicators, dynamically enable/disable links etc. Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
… links Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
…atures Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
Persist state betwee navigations so selections popover stays open or closed when user enters or leaves the same page. Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
Bare /log/:runId routes render the log-only layout because the page only shows tree or info panels when the mode query parameter explicitly requests them. The sidebar state was defaulting missing or unknown modes to treeAndinfoAndlog, so the main Log sidebar link could later reopen a different layout than the one the user visited. Normalize persisted log sidebar mode values and fall back to log for bare or invalid mode values.
When the run selection reached MAX_SELECTION_NUMBER, the table click handler removed the oldest run and then added the new run with two separate URL-backed state updates. Both callbacks computed from the same closed-over selectedRunIds value, so the add update could overwrite the removal and keep the oldest run selected. Add a replacement callback that computes the next selection array once and writes it with a single search-param update, then use it for the capped replacement path.
The history sidebar enabled Trend Charts and Series Charts when a saved chart URL existed, but the link target was rebuilt from the current history query. For queries without chart data, that sent users back to an unavailable chart view instead of the last valid saved chart page. Use the URLs resolved by useHistorySidebarState for those chart links so the existing saved-url fallback is honored. Add a regression test covering saved trend and series links without current chart data.
The admin sidebar refactor omitted the existing Analytics navigation entry. Deployments with analytics enabled still expose /admin/analytics, but admins lost the sidebar path to that route. Add the Analytics submenu item back behind the same gates used by the page: the current user must be an admin and server features must report analytics_enabled. Add regression coverage for the enabled, non-admin, and disabled-feature cases.
…URLs stripSidebarParamsFromUrl dropped every param whose value was the empty string, so a URL with explicitly-cleared filters (e.g. ?tagExpr=&runData=) lost those keys when persisted as the sidebar's last-visited URL. Restoring via the main nav link then re-defaulted the cleared filters. Only strip sidebar/project/default-mode params, not arbitrary empty-valued ones. Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
useRunTableQueryState forced location.state to undefined whenever the URL carried any persisted run-table param, which silently dropped a fresh dashboard 'unexpected' navigation intent when the target run link already contained expanded/globalFilter/rowState/columnFilters. A fresh click always carries a unique openUnexpectedIntentId, so honor it; keep suppressing only the legacy id-less booleans that can't be told apart from a reload. Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
The List and Charts sidebar submenu links were hardcoded to '/runs' and '/runs?mode=charts', so switching modes from a filtered runs view silently dropped tagExpr/dates/page. When on the runs page, build the target from the current search params (swapping only mode); when on another page, fall back to the persisted last-visited runs URL so unrelated params don't leak in. Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
migrateExpandedStateUrl built a URL with the migrated 'expanded' param but never wrote it back, so legacy dot-separated row-id state never migrated and the rewritten URL was discarded. Encode with the shared compressed format and apply it via history.replaceState. Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
syncChartsFromData colored restored charts by their position in the combinedPlots URL list, so any id missing from the loaded charts shifted every later chart's color away from the getColorByIdx(idx) value assigned at add time. Color by the chart's index in the full list instead. Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
linearUrl and aggregationUrl copied all current params including page, so switching from e.g. aggregation page 5 to linear could land on a non-existent page. Drop page when building these mode-switch URLs. Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
setEncodedUrlParam, encodeUrlForParam, parseModeParam and decodeUrlFromParam (the last a pure identity function) had no call sites anywhere in the codebase. Drop them to shrink the public surface of the sidebar feature. Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
log-sidebar-nav and measurements-sidebar-nav each defined an identical getModeFromSearch (URLSearchParams + switch) plus a duplicate local mode type. Extract a generic getModeFromSearch(search, allowedModes, default) into the sidebar lib and reuse the already-exported mode types. Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
The returned navigate closure was recreated on every render, so consumers that list it in a useCallback/useEffect dependency array (e.g. history-slice's handleFormChange, the selection-popover redirect effect) churned identity each render and gained no memoization. Wrap it in useCallback. Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
The six per-feature sidebar-state hooks (runs/run/history/log/measurements/ dashboard) each inlined the same write scaffold: read window.location.search, run updateSidebarStateSearchParams with a feature updater, bail if unchanged, then setSearchParams with replace + preserved location.state. Extract that mechanism into useSidebarStateWriter so it lives in one place; each hook now just supplies its key-writing updater. Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
okt-limonikas
force-pushed
the
sidebar-refactor
branch
from
2718731 to
1fd03d8
Compare
stripSidebarParamsFromUrl deliberately preserves empty-valued params (e.g. tagExpr=) per ef620d5; update the spec to match instead of expecting them dropped.
useRunSidebarState now writes via the shared useSidebarStateWriter hook, so the @/bublik/features/sidebar mock must export it. Replace the stale updateSidebarStateSearchParams mock with a useSidebarStateWriter stub.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
sidebar-navlibrary primitives.dashboard,runs,run,log,history,result,admin,help) and wires them through a newSidebarContainerin app layout.useNavigateWithProject,LinkWithProject, and sidebar param utilities).selection-popovercomponent in@/shared/tailwind-uiand updates dependent features.Key Changes
libs/bublik/features/sidebar-navwith reusable nav primitives (collapsible groups, submenu items, active matching, styles, utils, tests).main-nav/bottom-nav/nav-linkmodules inlibs/bublik/features/sidebarremoved and replaced with feature-composed navigation.apps/bublik/src/app/sidebar-container.tsx; related route/layout files updated.