fix: empty scene div causing dom flow to change if neither conditions are met #29109
Conversation
There was a problem hiding this comment.
PR Summary
Fixed an empty div rendering issue in the Navigation component and updated styling for input components to improve consistency and accessibility.
- Modified
/frontend/src/layout/navigation-3000/Navigation.tsxto only render notice container div when notices are present - Updated
/frontend/src/lib/lemon-ui/LemonSwitch/LemonSwitch.scssto use--bg-fill-mutedfor better contrast - Changed
/frontend/src/lib/lemon-ui/LemonCheckbox/LemonCheckbox.scssto use standardized input styling variables - Added
/frontend/src/lib/ui/Inputs.stories.tsxto showcase input components in Storybook with different backgrounds
7 file(s) reviewed, 4 comment(s)
Edit PR Review Bot Settings | Greptile
| {!sceneConfig?.hideProjectNotice && <ProjectNotice />} | ||
| </div> | ||
| {(!sceneConfig?.hideBillingNotice || !sceneConfig?.hideProjectNotice) && ( | ||
| <div className={sceneConfig?.layout === 'app-raw-no-header' ? 'px-4' : ''}> |
There was a problem hiding this comment.
style: Consider using Tailwind's conditional classes with clsx here instead of ternary operator for consistency with other className assignments in the file
| <LemonInput disabled /> | ||
| <LemonInput disabled /> |
There was a problem hiding this comment.
logic: Duplicate disabled LemonInput component with identical props
| <LemonInput disabled /> | |
| <LemonInput disabled /> | |
| <LemonInput disabled /> |
| <LemonInput disabled /> | ||
| <LemonInput disabled /> | ||
| <LemonInput value={"I'm pre-filled"} /> | ||
| <LemonSlider max={100} min={0} value={50} /> |
There was a problem hiding this comment.
logic: LemonSlider lacks onChange handler, making it non-interactive in Storybook
| } | ||
| export default meta | ||
|
|
||
| function GetAllInputs(): JSX.Element { |
There was a problem hiding this comment.
style: Function name 'GetAllInputs' does not follow camelCase convention used elsewhere in codebase
6ed961f to
02b760a
Compare
Problem
Changes
Hide the wrapping div if neither of the conditions are met.
Does this work well for both Cloud and self-hosted?
Should
How did you test this code?
Locally... which is difficult because locally we get the
This is a demo project with dummy data.project notice.