-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
feat(ourlogs): implement pinned logs with sticky header #115102
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
Open
JoshuaKGoldberg
wants to merge
18
commits into
master
Choose a base branch
from
ourlogs-pinned-logs-sticky
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
b93a496
feat(ourlogs): implement pinned logs with sticky header
JoshuaKGoldberg 3d4f3ca
Merge branch 'master' into ourlogs-pinned-logs-sticky
JoshuaKGoldberg 94ad61a
added feature flag / query param usage
JoshuaKGoldberg 42cc1a8
touchups: right actions; solid pin icons
JoshuaKGoldberg 744963c
touchups: that darn background
JoshuaKGoldberg 2e7bdcb
touchups: transition .fast
JoshuaKGoldberg be1e8a6
fix: most AI-reported bugs
JoshuaKGoldberg 254e2c6
fix: empty cell positioning
JoshuaKGoldberg 31352e4
why not, let's always give them background behind the scrollbar
JoshuaKGoldberg 58c4cd4
whoops, left in a testing condition
JoshuaKGoldberg 533caed
remove timestamp actions exemption
JoshuaKGoldberg 0627e89
test all the things
JoshuaKGoldberg 49713df
fix: add containsPin to BOLD_HOVER container
JoshuaKGoldberg 21d96c4
Make it not do anything without context
JoshuaKGoldberg 724547f
Remove linked hovering
JoshuaKGoldberg ef75c37
Merge branch 'master'
JoshuaKGoldberg a689e46
onCollapse/onExpand expansionKey, not rowId
JoshuaKGoldberg ca0602a
this single issue has convinced me that fine-grained reactivity is be…
JoshuaKGoldberg File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
135 changes: 135 additions & 0 deletions
135
static/app/views/explore/logs/pinning/PinnedLogs.spec.tsx
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,135 @@ | ||
| import {LogFixture} from 'sentry-fixture/log'; | ||
| import {OrganizationFixture} from 'sentry-fixture/organization'; | ||
|
|
||
| import { | ||
| render, | ||
| screen, | ||
| userEvent, | ||
| type RenderOptions, | ||
| } from 'sentry-test/reactTestingLibrary'; | ||
|
|
||
| import {PinnedLogs} from 'sentry/views/explore/logs/pinning/PinnedLogs'; | ||
| import {LogsPinningProvider} from 'sentry/views/explore/logs/pinning/useLogsPinning'; | ||
| import {OurLogKnownFieldKey} from 'sentry/views/explore/logs/types'; | ||
| import type {LogTableRowItem} from 'sentry/views/explore/logs/utils'; | ||
|
|
||
| const allRows: LogTableRowItem[] = [ | ||
| LogFixture({ | ||
| [OurLogKnownFieldKey.ID]: 'log-1', | ||
| [OurLogKnownFieldKey.PROJECT_ID]: '1', | ||
| [OurLogKnownFieldKey.ORGANIZATION_ID]: 1, | ||
| [OurLogKnownFieldKey.MESSAGE]: 'first pinned log', | ||
| }), | ||
| LogFixture({ | ||
| [OurLogKnownFieldKey.ID]: 'log-2', | ||
| [OurLogKnownFieldKey.PROJECT_ID]: '1', | ||
| [OurLogKnownFieldKey.ORGANIZATION_ID]: 1, | ||
| [OurLogKnownFieldKey.MESSAGE]: 'second pinned log', | ||
| }), | ||
| ]; | ||
|
|
||
| const renderRow = (dataRow: LogTableRowItem) => ( | ||
| <tr data-test-id={`pinned-row-${dataRow[OurLogKnownFieldKey.ID]}`}> | ||
| <td>{dataRow[OurLogKnownFieldKey.MESSAGE]}</td> | ||
| </tr> | ||
| ); | ||
|
|
||
| function renderPinnedLogs(options: RenderOptions = {}) { | ||
| return render( | ||
| <table> | ||
| <LogsPinningProvider> | ||
| <PinnedLogs allRows={allRows} renderRow={renderRow} /> | ||
| </LogsPinningProvider> | ||
| </table>, | ||
| { | ||
| organization: OrganizationFixture({features: ['ourlogs-pinning']}), | ||
| ...options, | ||
| } | ||
| ); | ||
| } | ||
|
|
||
| describe('PinnedLogs', () => { | ||
| it('renders nothing when the feature is disabled even if rows are pinned', () => { | ||
| renderPinnedLogs({ | ||
| organization: OrganizationFixture({features: []}), | ||
| initialRouterConfig: { | ||
| location: {pathname: '/', query: {logsPinned: 'log-1'}}, | ||
| }, | ||
| }); | ||
|
|
||
| expect(screen.queryByRole('toolbar')).not.toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('renders nothing when no rows are pinned', () => { | ||
| renderPinnedLogs(); | ||
|
|
||
| expect(screen.queryByRole('toolbar')).not.toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('renders the pinned row when its id is present in allRows', () => { | ||
| renderPinnedLogs({ | ||
| initialRouterConfig: { | ||
| location: {pathname: '/', query: {logsPinned: 'log-1'}}, | ||
| }, | ||
| }); | ||
|
|
||
| expect(screen.getByTestId('pinned-row-log-1')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('does not render a row when the pinned id is missing from allRows', () => { | ||
| renderPinnedLogs({ | ||
| initialRouterConfig: { | ||
| location: {pathname: '/', query: {logsPinned: 'missing-log'}}, | ||
| }, | ||
| }); | ||
|
|
||
| expect(screen.queryByTestId('pinned-row-missing-log')).not.toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('shows the count of pinned rows in the collapse toggle label', () => { | ||
| renderPinnedLogs({ | ||
| initialRouterConfig: { | ||
| location: {pathname: '/', query: {logsPinned: ['log-1', 'log-2']}}, | ||
| }, | ||
| }); | ||
|
|
||
| expect(screen.getByRole('button', {name: 'Collapse 2 pinned'})).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('hides the rendered pinned rows when the collapse button is clicked', async () => { | ||
| renderPinnedLogs({ | ||
| initialRouterConfig: { | ||
| location: {pathname: '/', query: {logsPinned: 'log-1'}}, | ||
| }, | ||
| }); | ||
|
|
||
| await userEvent.click(screen.getByRole('button', {name: 'Collapse 1 pinned'})); | ||
|
|
||
| expect(screen.queryByTestId('pinned-row-log-1')).not.toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('shows the rendered pinned rows again when the toggle button is clicked twice', async () => { | ||
| renderPinnedLogs({ | ||
| initialRouterConfig: { | ||
| location: {pathname: '/', query: {logsPinned: 'log-1'}}, | ||
| }, | ||
| }); | ||
|
|
||
| await userEvent.click(screen.getByRole('button', {name: 'Collapse 1 pinned'})); | ||
| await userEvent.click(screen.getByRole('button', {name: 'Expand 1 pinned'})); | ||
|
|
||
| expect(screen.getByTestId('pinned-row-log-1')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('removes the rendered pinned rows when the Clear all button is clicked', async () => { | ||
| renderPinnedLogs({ | ||
| initialRouterConfig: { | ||
| location: {pathname: '/', query: {logsPinned: 'log-1'}}, | ||
| }, | ||
| }); | ||
|
|
||
| await userEvent.click(screen.getByRole('button', {name: 'Clear all pins'})); | ||
|
|
||
| expect(screen.queryByTestId('pinned-row-log-1')).not.toBeInTheDocument(); | ||
| }); | ||
| }); |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| import {Fragment, useEffect, useState} from 'react'; | ||
| import styled from '@emotion/styled'; | ||
|
|
||
| import {Button} from '@sentry/scraps/button'; | ||
| import {Flex} from '@sentry/scraps/layout'; | ||
|
|
||
| import {GridRow} from 'sentry/components/tables/gridEditable/styles'; | ||
| import {IconChevron, IconClose} from 'sentry/icons'; | ||
| import {t} from 'sentry/locale'; | ||
| import {TableBody} from 'sentry/views/explore/components/table'; | ||
| import {useLogsPinning} from 'sentry/views/explore/logs/pinning/useLogsPinning'; | ||
| import {OurLogKnownFieldKey} from 'sentry/views/explore/logs/types'; | ||
| import type {LogTableRowItem} from 'sentry/views/explore/logs/utils'; | ||
|
|
||
| interface Props { | ||
| allRows: LogTableRowItem[]; | ||
| renderRow: (dataRow: LogTableRowItem) => React.ReactNode; | ||
| } | ||
|
|
||
| export function PinnedLogs({allRows, renderRow}: Props) { | ||
| const logsPinning = useLogsPinning(); | ||
| const [expanded, setExpanded] = useState(true); | ||
| const pinsCount = logsPinning?.pinnedRows.size; | ||
|
|
||
| useEffect(() => { | ||
| if (!pinsCount) { | ||
| setExpanded(true); | ||
| } | ||
| }, [pinsCount]); | ||
|
|
||
| if (!logsPinning || !pinsCount) { | ||
| return null; | ||
| } | ||
|
|
||
| return ( | ||
| <PinnedTableBody> | ||
| {expanded && | ||
| Array.from(logsPinning.pinnedRows).map(rowId => { | ||
| const dataRow = allRows.find(datum => datum[OurLogKnownFieldKey.ID] === rowId); | ||
|
|
||
| // TODO(LOGS-781): this is not correct yet because the virtualizer might not have found it yet. | ||
| // Will have to manually re-fetch data. | ||
| if (!dataRow) { | ||
|
JoshuaKGoldberg marked this conversation as resolved.
|
||
| return null; | ||
| } | ||
|
|
||
| return <Fragment key={rowId}>{renderRow(dataRow)}</Fragment>; | ||
| })} | ||
| <GridRow role="toolbar"> | ||
| <PinnedGridBodyCell> | ||
| <Flex justify="end"> | ||
| <Button | ||
| size="xs" | ||
| icon={<IconChevron size="xs" direction={expanded ? 'up' : 'down'} />} | ||
| onClick={() => setExpanded(previous => !previous)} | ||
| > | ||
| {expanded | ||
| ? t('Collapse %s pinned', pinsCount) | ||
| : t('Expand %s pinned', pinsCount)} | ||
| </Button> | ||
| <Button | ||
| aria-label={t('Clear all pins')} | ||
| icon={<IconClose size="xs" />} | ||
| onClick={logsPinning.clearPinnedRows} | ||
| size="xs" | ||
| variant="transparent" | ||
| > | ||
| {t('Clear all')} | ||
| </Button> | ||
| </Flex> | ||
| </PinnedGridBodyCell> | ||
| </GridRow> | ||
| </PinnedTableBody> | ||
| ); | ||
| } | ||
|
|
||
| const PinnedTableBody = styled(TableBody)` | ||
| border-bottom: 1px solid ${p => p.theme.tokens.border.primary}; | ||
| height: max-content; | ||
| overflow-y: auto; | ||
| overflow-x: hidden; | ||
| scrollbar-gutter: stable; | ||
| scrollbar-width: thin; | ||
| `; | ||
|
|
||
| const PinnedGridBodyCell = styled('td')` | ||
| grid-column: 1 / -1; | ||
| padding: ${p => p.theme.space.sm}; | ||
| `; | ||
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.