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
Changes from 1 commit
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
Next Next commit
fix: content layout shift fix, form label common component, and other…
… changes
  • Loading branch information
sashaboi committed May 24, 2024
commit 0c08369eb290ec740f2f570ba73334361a359a07
Original file line number Diff line number Diff line change
@@ -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">
13 changes: 7 additions & 6 deletions src/pages/AnalyticsPage/ChartSelectors/ChartSelectors.jsx
Original file line number Diff line number Diff line change
@@ -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) {
@@ -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}
@@ -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"
@@ -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}
@@ -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
Original file line number Diff line number Diff line change
@@ -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'

@@ -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
Original file line number Diff line number Diff line change
@@ -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
@@ -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}
Original file line number Diff line number Diff line change
@@ -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 />}>
13 changes: 6 additions & 7 deletions src/pages/RepoPage/CommitsTab/CommitsTab.jsx
Original file line number Diff line number Diff line change
@@ -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'
@@ -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}
@@ -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"
Original file line number Diff line number Diff line change
@@ -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'

@@ -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
11 changes: 5 additions & 6 deletions src/pages/RepoPage/CoverageTab/Summary/Summary.jsx
Original file line number Diff line number Diff line change
@@ -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'
@@ -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"
Original file line number Diff line number Diff line change
@@ -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'
@@ -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"
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'