Skip to content
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

fix: content layout shift fix, form label common component, and other changes #2899

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ function DetailsSection({ email, name }) {
}

return (
<form onSubmit={handleSubmit(submit)} className="mt-2 flex flex-col gap-4">
<form onSubmit={handleSubmit(submit)} className="flex flex-col gap-4">
<h3 className="text-lg font-semibold">Your details</h3>
<hr />
<div className="flex flex-col gap-1 md:w-1/2">
Expand Down
13 changes: 7 additions & 6 deletions src/pages/AnalyticsPage/ChartSelectors/ChartSelectors.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { useRepos } from 'services/repos'
import { TierNames, useTier } from 'services/tier'
import A from 'ui/A'
import DateRangePicker from 'ui/DateRangePicker'
import FormLabel from 'ui/FormLabel/FormLabel'
import MultiSelect from 'ui/MultiSelect'

function formatDataForMultiselect(repos) {
Expand All @@ -14,8 +15,8 @@ function formatDataForMultiselect(repos) {

function DateSelector({ startDate, endDate, updateParams }) {
return (
<div className="flex flex-col gap-2">
<span className="font-semibold">Dates</span>
<div className="flex flex-col gap-1">
<FormLabel label="Dates" />
<DateRangePicker
startDate={startDate}
endDate={endDate}
Expand Down Expand Up @@ -77,8 +78,8 @@ function RepoSelector({
}, [reposData?.pages])

return (
<div className="flex w-52 flex-col gap-2">
<span className="font-semibold">Repositories</span>
<div className="flex w-52 flex-col gap-1">
<FormLabel label="Repositories" />
<MultiSelect
hook="repo-chart-selector"
ariaName="Select repos to choose"
Expand Down Expand Up @@ -130,7 +131,7 @@ function ChartSelectors({ params, updateParams, active, sortItem }) {
}

return (
<div className="flex flex-wrap justify-center gap-4 border-b border-ds-gray-tertiary pb-4 sm:flex-nowrap sm:justify-start">
<div className="flex flex-wrap items-center justify-center gap-4 border-b border-ds-gray-tertiary pb-4 sm:flex-nowrap sm:justify-start">
<DateSelector
startDate={startDate}
endDate={endDate}
Expand All @@ -145,7 +146,7 @@ function ChartSelectors({ params, updateParams, active, sortItem }) {
resetRef={resetRef}
/>
<button
className="self-end text-ds-blue-darker md:mr-auto"
className="self-end pb-2 text-ds-blue-darker md:mr-auto"
onClick={clearFiltersHandler}
>
Clear filters
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ const columns = [
header: 'Asset',
cell: ({ getValue, row }) => {
return (
<p className="flex flex-row break-all">
<p className="flex flex-row items-center break-all">
<span
data-action="clickable"
data-testid="modules-expand"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { Branch, useBranch, useBranches } from 'services/branches'
import { useNavLinks } from 'services/navigation'
import { useRepoOverview } from 'services/repo'
import A from 'ui/A'
import FormLabel from 'ui/FormLabel/FormLabel'
import Icon from 'ui/Icon'
import Select from 'ui/Select'

Expand Down Expand Up @@ -87,13 +88,11 @@ const BranchSelector: React.FC<BranchSelectorProps> = ({
}

return (
<div className="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" />
</span>
Branch Context
</h3>
<div className="flex flex-col gap-1 md:w-[16rem]">
<FormLabel
label="Branch Context"
icon={<Icon name="branch" size="sm" variant="developer" />}
/>
<span className="min-w-[16rem] text-sm">
<Select
// @ts-expect-error - Select has some TS issues because it's still written in JS
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,13 @@ import { useHistory, useParams } from 'react-router-dom'
import { useBranchBundlesNames } from 'services/bundleAnalysis'
import { useNavLinks } from 'services/navigation'
import { useRepoOverview } from 'services/repo'
import FormLabel from 'ui/FormLabel/FormLabel'
import Select from 'ui/Select'

export const BundleSelectorSkeleton: React.FC = () => {
return (
<div className="md:w-[16rem]">
<h3 className="flex items-center gap-1 text-sm font-semibold text-ds-gray-octonary">
Bundle
</h3>
<div className="flex flex-col gap-1 md:w-[16rem]">
<FormLabel label="Bundle" />
<span className="max-w-[16rem] text-sm">
<Select
// @ts-expect-error
Expand Down Expand Up @@ -86,10 +85,8 @@ const BundleSelector = forwardRef(({}, ref) => {
const [filteredBundles, setFilteredBundles] = useState<Array<string>>([])

return (
<div className="md:w-[16rem]">
<h3 className="flex items-center gap-1 text-sm font-semibold text-ds-gray-octonary">
Bundle
</h3>
<div className="flex flex-col gap-1 md:w-[16rem]">
<FormLabel label="Bundle" />
<span className="max-w-[16rem] text-sm">
<Select
ref={ref}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const BundleSummary: React.FC = () => {
}, [])

return (
<div className="flex flex-col gap-8 py-4 md:flex-row md:justify-between">
<div className="flex flex-col gap-8 pb-4 md:flex-row md:justify-between">
<div className="flex flex-col gap-4 md:flex-row">
<BranchSelector resetBundleSelect={resetBundleSelect} />
<Suspense fallback={<BundleSelectorSkeleton />}>
Expand Down
13 changes: 6 additions & 7 deletions src/pages/RepoPage/CommitsTab/CommitsTab.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { useBranchHasCommits } from 'services/branches'
import { useLocationParams } from 'services/navigation'
import { useRepoOverview, useRepoSettingsTeam } from 'services/repo'
import { TierNames, useTier } from 'services/tier'
import FormLabel from 'ui/FormLabel/FormLabel'
import Icon from 'ui/Icon'
import MultiSelect from 'ui/MultiSelect'
import SearchField from 'ui/SearchField'
Expand Down Expand Up @@ -158,12 +159,10 @@ function CommitsTab() {
<div className="grid grid-cols-1 justify-between gap-4 md:grid-cols-2 md:items-end md:gap-0">
<div className="flex flex-col gap-2 px-2 sm:px-0 md:flex-row">
<div className="flex flex-col gap-1">
<h2 className="flex flex-initial items-center gap-1 font-semibold">
<span className="text-ds-gray-quinary">
<Icon name="branch" variant="developer" size="sm" />
</span>
Branch Context
</h2>
<FormLabel
label="Branch Context"
icon={<Icon name="branch" size="sm" variant="developer" />}
/>
<div className="min-w-[13rem] lg:min-w-[16rem]">
<Select
{...branchSelectorProps}
Expand All @@ -187,7 +186,7 @@ function CommitsTab() {
</div>
</div>
<div className="flex flex-col gap-1">
<h2 className="font-semibold">CI status</h2>
<FormLabel label="CI status" />
<div className="min-w-[13rem] lg:min-w-[16rem]">
<MultiSelect
dataMarketing="commits-filter-by-status"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { Branch, useBranch, useBranches } from 'services/branches'
import { useLocationParams } from 'services/navigation'
import { useRepoOverview } from 'services/repo'
import A from 'ui/A'
import FormLabel from 'ui/FormLabel/FormLabel'
import Icon from 'ui/Icon'
import Select from 'ui/Select'

Expand Down Expand Up @@ -81,12 +82,10 @@ const BranchSelector: React.FC<BranchSelectorProps> = ({

return (
<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" />
</span>
Branch Context
</h3>
<FormLabel
label="Branch Context"
icon={<Icon name="branch" size="sm" variant="developer" />}
/>
<span className="min-w-[16rem] text-sm">
<Select
// @ts-expect-error - select is not typed
Expand Down
11 changes: 5 additions & 6 deletions src/pages/RepoPage/CoverageTab/Summary/Summary.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { useRepoConfig } from 'services/repo/useRepoConfig'
import { determineProgressColor } from 'shared/utils/determineProgressColor'
import A from 'ui/A'
import CoverageProgress from 'ui/CoverageProgress'
import FormLabel from 'ui/FormLabel/FormLabel'
import Icon from 'ui/Icon'
import Select from 'ui/Select'
import { SummaryField, SummaryRoot } from 'ui/Summary'
Expand Down Expand Up @@ -62,12 +63,10 @@ const Summary = () => {
)}
<SummaryRoot>
<SummaryField>
<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" />
</span>
Branch Context
</h3>
<FormLabel
label="Branch Context"
icon={<Icon name="branch" size="sm" variant="developer" />}
/>
<span className="min-w-[16rem] text-sm">
<Select
dataMarketing="branch-selector-repo-page"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { useParams } from 'react-router-dom'
import { useBranches } from 'services/branches'
import { useUpdateRepo } from 'services/repo'
import { useAddNotification } from 'services/toastNotification'
import FormLabel from 'ui/FormLabel/FormLabel'
import Icon from 'ui/Icon'
import Select from 'ui/Select'
import SettingsDescriptor from 'ui/SettingsDescriptor'
Expand Down Expand Up @@ -58,10 +59,10 @@ function DefaultBranch({ defaultBranch }) {
description="Selection for branch context of data in coverage dashboard"
content={
<>
<h2 className="flex gap-1 font-semibold">
<Icon name="branch" variant="developer" size="sm" />
Branch Context
</h2>
<FormLabel
label="Branch Context"
icon={<Icon name="branch" size="sm" variant="developer" />}
/>
<div className="grid grid-cols-2">
<Select
dataMarketing="branch-selector-settings-tab"
Expand Down
17 changes: 17 additions & 0 deletions src/ui/FormLabel/FormLabel.spec.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { render, screen } from '@testing-library/react'
import React from 'react'

import FormLabel from './FormLabel'

describe('FormLabel', () => {
it('should render label if label is provided, and icon should not render', () => {
render(<FormLabel label="Form label placeholder" />)
expect(screen.getByText('Form label placeholder')).toBeInTheDocument()
expect(screen.queryByTestId('form-label-icon')).not.toBeInTheDocument()
})
it('should render label and icon if provided', () => {
render(<FormLabel label="Form label placeholder" icon={<div>ICON</div>} />)
expect(screen.getByText('Form label placeholder')).toBeInTheDocument()
expect(screen.getByTestId('form-label-icon')).toBeInTheDocument()
})
})
21 changes: 21 additions & 0 deletions src/ui/FormLabel/FormLabel.tsx
Copy link
Contributor

Choose a reason for hiding this comment

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

Wondering @sashaboi if you're alright refactoring this component to a new composable style that we've introduced here at Codecov. Here's an example, if you have any questions let me know.

Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import React, { ReactNode } from 'react'

interface FormLabelProps {
label: string
icon?: ReactNode
}

const FormLabel: React.FC<FormLabelProps> = ({ label, icon }) => {
return (
<h3 className="flex items-center gap-1 text-sm font-semibold text-ds-gray-octonary">
{icon && (
<span data-testid="form-label-icon" className="text-ds-gray-quinary">
{icon}
</span>
)}
{label}
</h3>
)
}

export default FormLabel
1 change: 1 addition & 0 deletions src/ui/FormLabel/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { default } from './FormLabel'
Loading