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: empty scene div causing dom flow to change if neither conditions are met #29109

Closed
wants to merge 2 commits into from
Closed
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
11 changes: 7 additions & 4 deletions frontend/src/layout/navigation-3000/Navigation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,13 @@ export function Navigation({
sceneConfig?.layout === 'app-raw-no-header' && 'Navigation3000__scene--raw-no-header'
)}
>
<div className={sceneConfig?.layout === 'app-raw-no-header' ? 'px-4' : ''}>
{!sceneConfig?.hideBillingNotice && <BillingAlertsV2 />}
{!sceneConfig?.hideProjectNotice && <ProjectNotice />}
</div>
{(!sceneConfig?.hideBillingNotice || !sceneConfig?.hideProjectNotice) && (
<div className={sceneConfig?.layout === 'app-raw-no-header' ? 'px-4' : ''}>
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Consider using Tailwind's conditional classes with clsx here instead of ternary operator for consistency with other className assignments in the file

{!sceneConfig?.hideBillingNotice && <BillingAlertsV2 />}
{!sceneConfig?.hideProjectNotice && <ProjectNotice />}
</div>
)}

{children}
</div>
</main>
Expand Down
8 changes: 4 additions & 4 deletions frontend/src/lib/lemon-ui/LemonCheckbox/LemonCheckbox.scss
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,13 @@
flex-shrink: 0;
width: 1rem;
height: 1rem;
background: var(--bg-surface-primary);
border: 1.5px solid var(--border-bold);
background: var(--bg-fill-input);
border: 1.5px solid var(--border-primary);
border-radius: 0.25rem; // Intentionally a bit smaller than --radius
transition: border 200ms ease, background 200ms ease;

path {
stroke: var(--bg-surface-primary);
stroke: var(--text-primary-inverse);
stroke-dasharray: var(--tick-length);
stroke-dashoffset: var(--tick-length);
transition: stroke-dashoffset 200ms ease;
Expand Down Expand Up @@ -98,7 +98,7 @@
label {
min-height: var(--lemon-checkbox-height);
padding: 0 0.75rem;
background: var(--bg-surface-primary);
background: var(--bg-fill-input);
border: 1px solid var(--border-primary);
border-radius: var(--radius);
}
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/lib/lemon-ui/LemonSlider/LemonSlider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ export function LemonSlider({ value = 0, onChange, min, max, step = 1, className
setDragging(true)
}}
>
<div className="w-full bg-fill-slider-rail rounded-full h-[6px]" />
<div className="w-full bg-fill-muted rounded-full h-[6px]" />
</div>
<div
className="absolute h-[6px] bg-accent-primary rounded-full pointer-events-none"
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/lib/lemon-ui/LemonSwitch/LemonSwitch.scss
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@
width: 100%;
height: 100%;
pointer-events: none;
background-color: var(--bg-fill-switch);
background-color: var(--bg-fill-muted);
border: 1px solid var(--border-primary);
border-radius: var(--lemon-switch-height);
transition: background-color 100ms ease;
Expand Down
64 changes: 64 additions & 0 deletions frontend/src/lib/ui/Inputs.stories.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import { LemonCheckbox, LemonSwitch } from '@posthog/lemon-ui'
import { Meta } from '@storybook/react'
import { LemonInput } from 'lib/lemon-ui/LemonInput/LemonInput'
import { LemonRadio } from 'lib/lemon-ui/LemonRadio/LemonRadio'
import { LemonSlider } from 'lib/lemon-ui/LemonSlider/LemonSlider'
import { useState } from 'react'

const meta: Meta = {
title: 'Design System/Inputs',
tags: ['autodocs'],
}
export default meta

function GetAllInputs(): JSX.Element {
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Function name 'GetAllInputs' does not follow camelCase convention used elsewhere in codebase

const [checked, setChecked] = useState(false)
const [radioValue, setRadioValue] = useState('this')

return (
<div className="flex flex-col gap-2">
<LemonInput />
<LemonInput placeholder="Placeholder" />
<LemonInput disabled />
<LemonInput disabled />
Comment on lines +22 to +23
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Duplicate disabled LemonInput component with identical props

Suggested change
<LemonInput disabled />
<LemonInput disabled />
<LemonInput disabled />

<LemonInput value={"I'm pre-filled"} />
<LemonSlider max={100} min={0} value={50} />
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: LemonSlider lacks onChange handler, making it non-interactive in Storybook

<LemonCheckbox label="Checkbox" />
<LemonCheckbox label="Checkbox" disabledReason="This is not the way to Amarillo" />
<LemonCheckbox label="Checkbox" checked />
<LemonRadio
value={radioValue}
options={[
{ label: 'This', value: 'this' },
{ label: 'That', value: 'that' },
]}
onChange={setRadioValue}
/>
<LemonRadio
value={radioValue}
options={[
{ label: 'This', value: 'this', disabledReason: 'This is not the way to Amarillo' },
{ label: 'That', value: 'that' },
]}
onChange={setRadioValue}
/>
<LemonSwitch checked={checked} onChange={() => setChecked(!checked)} />
<LemonSwitch checked={checked} bordered onChange={() => setChecked(!checked)} />
<LemonSwitch checked={true} disabledReason="This is not the way to Amarillo" />
<LemonSwitch checked={false} />
<LemonSwitch checked={false} bordered />
<LemonSwitch checked={false} disabledReason="This is not the way to Amarillo" />
</div>
)
}

export const AllInputs = (): JSX.Element => {
return (
<div className="space-y-2">
<div className="bg-primary p-4">{GetAllInputs()}</div>
<div className="bg-surface-primary p-4">{GetAllInputs()}</div>
<div className="bg-surface-secondary p-4">{GetAllInputs()}</div>
<div className="bg-surface-tertiary p-4">{GetAllInputs()}</div>
</div>
)
}
6 changes: 4 additions & 2 deletions frontend/src/styles/base.scss
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,9 @@
--bg-fill-secondary: var(--primitive-3000-25);
--bg-fill-tertiary: var(--primitive-3000-50);

// A fill to always ensure contrast
--bg-fill-muted: color-mix(in oklab, var(--color-black) 10%, transparent);

// Highlights
--bg-fill-primary-highlight: color-mix(
in oklab,
Expand Down Expand Up @@ -437,7 +440,6 @@
// Inputs
--bg-fill-input: var(--color-white);
--bg-fill-switch: var(--primitive-neutral-cool-250);
--bg-fill-slider-rail: color-mix(in oklab, var(--color-black) 10%, transparent);

// Content (text, icons)
// //////////////////////////////////////////////////////////
Expand Down Expand Up @@ -822,6 +824,7 @@
--bg-fill-primary: var(--primitive-neutral-cool-900);
--bg-fill-secondary: var(--primitive-neutral-cool-850);
--bg-fill-tertiary: var(--primitive-neutral-cool-800);
--bg-fill-muted: color-mix(in oklab, var(--color-white) 15%, transparent);
--bg-fill-info-secondary: var(--color-blue-950);
--bg-fill-info-tertiary: var(--color-blue-950);
--bg-fill-warning-secondary: var(--color-yellow-950);
Expand All @@ -836,7 +839,6 @@
--bg-fill-primary-highlight: color-mix(in oklab, var(--color-primary-950) 50%, transparent);
--bg-fill-input: var(--primitive-neutral-cool-850);
--bg-fill-switch: var(--primitive-neutral-cool-900);
--bg-fill-slider-rail: color-mix(in oklab, var(--color-white) 15%, transparent);

// Content (text, icons) (dark mode)
// //////////////////////////////////////////////////////////
Expand Down
3 changes: 2 additions & 1 deletion frontend/tailwind.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,7 @@ const config = {
'fill-primary': 'var(--bg-fill-primary)',
'fill-secondary': 'var(--bg-fill-secondary)',
'fill-tertiary': 'var(--bg-fill-tertiary)',
'fill-muted': 'var(--bg-fill-muted)',
'fill-primary-highlight': 'var(--bg-fill-primary-highlight)',
'fill-info-secondary': 'var(--bg-fill-info-secondary)',
'fill-info-tertiary': 'var(--bg-fill-info-tertiary)',
Expand All @@ -648,7 +649,7 @@ const config = {
'fill-success-highlight': 'var(--bg-fill-success-highlight)',
'fill-input': 'var(--bg-fill-input)',
'fill-switch': 'var(--bg-fill-switch)',
'fill-slider-rail': 'var(--bg-fill-slider-rail)',
'fill-checkbox': 'var(--bg-fill-checkbox)',
},
textColor: {
...commonColors,
Expand Down
Loading