Skip to content

Commit

Permalink
Assortment of UI changes for coverage over time for components (#2809)
Browse files Browse the repository at this point in the history
- Add a link to redirect from components tab on PR page to components tab on Repo page
- Add latest commit under the branch selector
- Disable the branch dropdown box on the not_configured/enable/backfilling views
- When clicking "Enable component analytics" button it should reload the page (which would bring it to backfilling view)
- Fix padding on the dropdowns section. The padding between the vertical line and dropdown and padding between the dropdown label and dropbown
- Add table headers when showing the not_configured/enable/backfilling views
- Last Uploaded table header row values should align to the left
- The delete component modal message is not correct
- Add a aria-label = "delete-{component_id}" to the trash can icon to delete
- Update the message from 24 hour to 1 hour on the backfilling view
  • Loading branch information
JerrySentry authored Apr 26, 2024
1 parent 577c127 commit 9f18102
Show file tree
Hide file tree
Showing 18 changed files with 177 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ const mockPull = {
},
],
},
head: {
branchName: 'abc',
},
},
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import qs, { type ParsedQs } from 'qs'
import { useMemo } from 'react'
import { useLocation } from 'react-router-dom'

import A from 'ui/A'
import Spinner from 'ui/Spinner'
import TotalsNumber from 'ui/TotalsNumber'

Expand Down Expand Up @@ -132,7 +133,17 @@ export default function ComponentsTable() {

return (
<>
<div className="my-2 flex flex-row-reverse">
<div className="flex w-full items-center justify-between px-5 py-2">
<A
hook="components"
to={{
pageName: 'componentsTab',
options: { branch: data?.pull?.head?.branchName },
}}
isExternal
>
View components over time
</A>
<ComponentsSelector />
</div>
<div className="tableui">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ export const query = `
message
}
}
head {
branchName
}
}
}
... on NotFoundError {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ const mockResponse = {
},
],
},
head: {
branchName: 'abc',
},
},
},
},
Expand Down Expand Up @@ -139,6 +142,9 @@ describe('useComponentComparison', () => {
},
],
},
head: {
branchName: 'abc',
},
},
})
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ const RepositorySchema = z.object({
MissingHeadCommitSchema,
MissingHeadReportSchema,
]),
head: z
.object({
branchName: z.string().nullable(),
})
.nullable(),
})
.nullable(),
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ describe('SyncingBanner', () => {
it('renders heading and content components', () => {
render(<SyncingBanner />, { wrapper })
const historicalDataTextLong = screen.getByText(
'It might take up to 24 hours to view your data.'
'It might take up to 1 hour to view your data.'
)
expect(historicalDataTextLong).toBeInTheDocument()
})
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
import Spinner from 'ui/Spinner'

import { LoadingTable } from '../../subroute/ComponentsTable/ComponentsTable'

function SyncingBanner() {
return (
<div className="mt-12 grid gap-4">
<div className="grid gap-4 pt-4">
<LoadingTable />
<div className="flex flex-col items-center gap-1">
<Spinner />
<p>It might take up to 24 hours to view your data.</p>
<p>It might take up to 1 hour to view your data.</p>
</div>
</div>
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { useParams } from 'react-router-dom'
import { MEASUREMENT_TYPE, useActivateMeasurements } from 'services/repo'
import Button from 'ui/Button'

import { LoadingTable } from '../../subroute/ComponentsTable/ComponentsTable'

type URLParams = {
provider: string
owner: string
Expand All @@ -19,7 +21,8 @@ function TriggerSyncBanner() {
})

return (
<div className="mt-12 grid gap-4">
<div className="grid gap-4 pt-4">
<LoadingTable />
<div className="flex flex-col items-center gap-1">
<p>No data to display</p>
<p>You will need to enable components to see related coverage data.</p>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ describe('BranchSelector', () => {
describe('with populated data', () => {
it('renders the branch selector', async () => {
const { queryClient } = setup()
render(<BranchSelector />, {
render(<BranchSelector isDisabled={false} />, {
wrapper: wrapper(queryClient),
})

Expand All @@ -213,7 +213,7 @@ describe('BranchSelector', () => {

it('renders default branch as selected branch', async () => {
const { queryClient } = setup()
render(<BranchSelector />, {
render(<BranchSelector isDisabled={false} />, {
wrapper: wrapper(queryClient),
})

Expand All @@ -226,7 +226,7 @@ describe('BranchSelector', () => {
describe('user selects a branch', () => {
it('updates params with selected branch', async () => {
const { user, queryClient } = setup()
render(<BranchSelector />, {
render(<BranchSelector isDisabled={false} />, {
wrapper: wrapper(queryClient),
})

Expand Down Expand Up @@ -258,7 +258,7 @@ describe('BranchSelector', () => {
isIntersecting: true,
})

render(<BranchSelector />, {
render(<BranchSelector isDisabled={false} />, {
wrapper: wrapper(queryClient),
})

Expand All @@ -281,7 +281,7 @@ describe('BranchSelector', () => {
isIntersecting: true,
})

render(<BranchSelector />, {
render(<BranchSelector isDisabled={false} />, {
wrapper: wrapper(queryClient),
})

Expand All @@ -298,7 +298,7 @@ describe('BranchSelector', () => {
describe('user searches for branch', () => {
it('calls the api with the search value', async () => {
const { mockSearching, user, queryClient } = setup()
render(<BranchSelector />, {
render(<BranchSelector isDisabled={false} />, {
wrapper: wrapper(queryClient),
})

Expand All @@ -319,7 +319,7 @@ describe('BranchSelector', () => {
const { queryClient } = setup({
nullOverview: true,
})
render(<BranchSelector />, {
render(<BranchSelector isDisabled={false} />, {
wrapper: wrapper(queryClient),
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { useParams } from 'react-router-dom'
import { Branch, useBranch, useBranches } from 'services/branches'
import { useLocationParams } from 'services/navigation'
import { useRepoOverview } from 'services/repo'
import A from 'ui/A'
import Icon from 'ui/Icon'
import Select from 'ui/Select'

Expand All @@ -21,7 +22,13 @@ const defaultQueryParams = {
const getDecodedBranch = (branch?: string) =>
!!branch ? decodeURIComponent(branch) : undefined

const BranchSelector: React.FC = () => {
interface BranchSelectorProps {
isDisabled: boolean | undefined
}

const BranchSelector: React.FC<BranchSelectorProps> = ({
isDisabled = false,
}) => {
const { provider, owner, repo, branch } = useParams<URLParams>()
const { params, updateParams } = useLocationParams(defaultQueryParams)
const [branchSearchTerm, setBranchSearchTerm] = useState<string>('')
Expand All @@ -48,7 +55,10 @@ const BranchSelector: React.FC = () => {
})

const decodedBranch = getDecodedBranch(branch)
const selectedBranch = decodedBranch ?? overview?.defaultBranch ?? ''

const selectedBranch =
// @ts-expect-error
decodedBranch || params.branch || overview?.defaultBranch || ''

const { data: searchBranchValue } = useBranch({
provider,
Expand All @@ -70,7 +80,7 @@ const BranchSelector: React.FC = () => {
}

return (
<div className="md:w-[16rem]">
<div className="flex flex-col justify-between gap-2 p-4 sm:py-0 md:w-[16rem]">
<h3 className="flex items-center gap-1 text-sm font-semibold text-ds-gray-octonary">
<span className="text-ds-gray-quinary">
<Icon name="branch" size="sm" variant="developer" />
Expand All @@ -97,8 +107,24 @@ const BranchSelector: React.FC = () => {
}
}}
onSearch={(term: string) => setBranchSearchTerm(term)}
disabled={isDisabled}
/>
</span>
{selection?.head?.commitid && (
<p className="text-xs">
<span className="font-bold">Source:</span> latest commit{' '}
<A
to={{
pageName: 'commit',
options: { commit: selection?.head?.commitid },
}}
isExternal={false}
hook="components-latest-commit"
>
{selection?.head?.commitid.slice(0, 7)}
</A>
</p>
)}
</div>
)
}
Expand Down
8 changes: 4 additions & 4 deletions src/pages/RepoPage/ComponentsTab/Header/Header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ const Header = ({
return (
<div className="flex flex-col justify-end divide-y divide-solid divide-ds-gray-secondary">
<div className="grid w-2/3 divide-y divide-solid divide-ds-gray-secondary sm:w-full sm:grid-cols-2 sm:divide-x sm:divide-y-0 md:grid-cols-4">
<BranchSelector />
<div className="md:w-[16rem]">
<BranchSelector isDisabled={controlsDisabled} />
<div className="flex flex-col justify-between gap-2 p-4 sm:py-0">
<h3 className="text-sm font-semibold text-ds-gray-octonary">
Configured components
</h3>
Expand All @@ -73,7 +73,7 @@ const Header = ({
<A to={{ pageName: 'components' }}>Learn more</A>
</p>
</div>
<div className="md:w-[16rem]">
<div className="flex flex-col gap-2 p-4 sm:py-0">
<h3 className="text-sm font-semibold text-ds-gray-octonary">
Historical trend
</h3>
Expand All @@ -91,7 +91,7 @@ const Header = ({
renderSelected={({ label }: { label: string }) => label}
/>
</div>
<div className="md:w-[16rem]">
<div className="flex flex-col gap-2 p-4 sm:py-0">
<h3 className="text-sm font-semibold text-ds-gray-octonary">
Show by
</h3>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,9 +275,7 @@ describe('ComponentsTable', () => {
const { user } = setup({})
render(<ComponentsTable />, { wrapper: wrapper() })

const trashIconButtons = await screen.findAllByRole('button', {
name: /trash/,
})
const trashIconButtons = await screen.findAllByTestId('delete-component')
expect(trashIconButtons).toHaveLength(3)

const [firstIcon] = trashIconButtons
Expand All @@ -287,8 +285,8 @@ describe('ComponentsTable', () => {
}
})

const deleteComponentModalText = await screen.findByText(
'Delete Component'
const deleteComponentModalText = await screen.findByTestId(
'delete-component-modal'
)
expect(deleteComponentModalText).toBeInTheDocument()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,50 @@ interface ComponentsTableHelper {
delete: React.ReactElement | null
}

export const LoadingTable = () => {
const data = useMemo(() => [], [])
const table = useReactTable({
data,
columns,
getCoreRowModel: getCoreRowModel(),
})

return (
<div className="tableui">
<table>
<colgroup>
<col className="@sm/table:w-4/12" />
<col className="@sm/table:w-3/12" />
<col className="@sm/table:w-3/12" />
<col className="@sm/table:w-2/12" />
<col className="@sm/table:w-3/12" />
</colgroup>
<thead data-testid="header-row">
{table.getHeaderGroups().map((headerGroup) => (
<tr key={headerGroup.id}>
{headerGroup.headers.map((header) => (
<th key={header.id}>
<div
className={cs('flex flex-row', {
'justify-end':
header.id !== 'name' && header.id !== 'lastUploaded',
})}
>
{flexRender(
header.column.columnDef.header,
header.getContext()
)}
</div>
</th>
))}
</tr>
))}
</thead>
</table>
</div>
)
}

const columnHelper = createColumnHelper<ComponentsTableHelper>()

const columns = [
Expand Down Expand Up @@ -125,7 +169,7 @@ function createTableData({
/>
),
lastUploaded: (
<span className="flex justify-end">
<span className="flex justify-start">
{lastUploaded ? formatTimeToNow(lastUploaded) : ''}
</span>
),
Expand All @@ -135,6 +179,7 @@ function createTableData({
data-testid="delete-component"
onClick={() => setModalInfo({ componentId: name, showModal: true })}
className="text-ds-gray-tertiary hover:text-ds-gray-senary"
aria-label={'delete ' + name}
>
<Icon size="md" name="trash" variant="outline" />
</button>
Expand Down Expand Up @@ -199,7 +244,8 @@ const ComponentTable = memo(function Table({
>
<div
className={cs('flex flex-row', {
'justify-end': header.id !== 'name',
'justify-end':
header.id !== 'name' && header.id !== 'lastUploaded',
})}
>
{flexRender(
Expand Down
Loading

0 comments on commit 9f18102

Please sign in to comment.