feat(releases): Use routing for Releases Drawer #88562
Merged
Conversation
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
github-actions
bot
added
the
Scope: Frontend
billyvg
force-pushed
the
billy/replay-2-releases-drawer-needs-to-have-deeplinks
branch
from
5b45118 to
42c868c
Compare
billyvg
force-pushed
the
billy/replay-2-releases-drawer-needs-to-have-deeplinks
branch
from
42c868c to
f6cde8e
Compare
billyvg
commented
Apr 7, 2025
billyvg
force-pushed
the
billy/replay-2-releases-drawer-needs-to-have-deeplinks
branch
from
dd5465a to
e03efe2
Compare
billyvg
commented
Apr 8, 2025
billyvg
force-pushed
the
billy/replay-2-releases-drawer-needs-to-have-deeplinks
branch
from
4847126 to
d7dced6
Compare
|
The chart rendering hack breaks the breadcrumb navigation from release details. Going to revert this back and wait until we get chart deeplinking. |
billyvg
added a commit
that referenced
this pull request
Apr 24, 2025
) These hooks are using `useLocation` to spread the entire `location.query` object (less a few fields) to the API request. Instead, directly use pageFilters instead of the current location query object. Required for #88562
billyvg
added a commit
that referenced
this pull request
Apr 24, 2025
Refactors the Insights -> LLM charts to better support deep linking in a [future PR](#88562). Notable in this PR is that these widgets are used in 3 different sections: 1) Landing Page - no additional args 2) Details Page - takes a `groupId` from URL parameters 3) "LLM Monitor Section" (group details page) - The wrapping components takes in an `event` argument, which uses the trace (span id + trace id) to query for a groupId to pass to widgets We're going to refactor the 3 widgets into: - ~(3) base widgets that accept an optional `groupId` (referring to AI Pipeline Group, and not our normal Issues Group)~ - (3) widgets that do not require a groupId from URL params. These widgets handle point 1) above - (3) widgets that use `groupId` from URL parameters. These widgets handle points ~1) and~ 2) above. - (2) widgets (`llmEvent*`) that take `groupId` (Issues Group!) and `eventId` from URL parameters to fetch the Issues Group, and then tries to find the AI Pipeline Group id via the event's trace. This AI Pipeline Group ID is then used in the query and renders the appropriate charts. These widgets account for point 3) above. ~In order to handle this[^1] (and for future widgets), this PR introduces a `chartProperties` prop for `LoadableChartWidgetProps`. These are to be used as URL query params when linking to the Releases Drawer, but also used to render the chart as well. They are chart-specific, which is the reason for the additional level of object nesting. Think of it as these two stages:~ ~- "Serialization": we have a chart, and want to "serialize" it into a URL. This will involve 2 steps~ ~- taking the *keys* of `chartProperties` object and using a known query parameter key (e.g. `rdChartPropertyKey`) to create a list of query parameters (e.g. `rdChartPropertyKey=foo&rdChartPropertyKey=bar`)~ ~- turning `chartProperties` into query parameters (e.g. `foo=val1&bar=val2`)~ ~- "Parsing": inverse of "serialization" -- we have a URL and want to render a chart.~ ~- using known key (`rdChartPropertyKey`), we're able to fetch the keys of `chartProperties` (which can vary from chart to chart)~ ~- with the keys, we can again look through the query parameters to get the values for each key~ ~The known key (e.g. `rdChartPropertKey`) acts as an indirect pointer to the `chartProperties` object which has dynamic keys. See [this commit](00d9fd6#diff-0269df2be334787a8238014bd4dae87472c3adf1b5f8021a7a151fc464427c3fR33-R42) for an example.~ Part of #88560 [^1]: There is another solution, since (1) and (2) can be handled without any changes, we would just need slightly diff widgets for (3) where it's mostly the same except it expects `eventId` to available via URL params and it fetches the event to get the trace and then fetches the group id
michellewzhang
approved these changes
Apr 24, 2025
billyvg
deleted the
billy/replay-2-releases-drawer-needs-to-have-deeplinks
branch
evanh
pushed a commit
that referenced
this pull request
Apr 25, 2025
This allows the Releases drawer to be deeplinked (including charts). This splits `<ReleasesDrawer>` into `useReleasesDrawer()` hook which opens drawer and renders `<ReleasesDrawerList>` or `<ReleasesDrawerDetails>`.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Note: this does not currently handle charts inside of the drawer list.
This splits
<ReleasesDrawer>intouseReleasesDrawer()hook which opens drawer and renders<ReleasesDrawerList>or<ReleasesDrawerDetails>.