Skip to content

[Chore] Minor CSS changes for error overlay in feature tree #6977

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

[Chore] Minor CSS changes for error overlay in feature tree #6977

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e6dd628
fix: red error overlay fills container, does not have rounded edges a…
nadr0 May 15, 2025
625394d
fix: show the errored background in the feature tree if there is a a …
nadr0 May 15, 2025
8d19a95
fix: minor css nits, cannot figure out how to make this button cleaner
nadr0 May 15, 2025
939d5ef
fix: reverted my testing code, needed to use this to check the colors
nadr0 May 15, 2025
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
12 changes: 7 additions & 5 deletions src/components/ModelingSidebar/ModelingPanes/FeatureTreePane.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ export const FeatureTreePane = () => {
// devTools: true,
}
)

// If there are parse errors we show the last successful operations
// and overlay a message on top of the pane
const parseErrors = kclManager.errors.filter((e) => e.kind !== 'engine')
Expand Down Expand Up @@ -160,20 +161,21 @@ export const FeatureTreePane = () => {
{!modelingState.matches('Sketch') && <DefaultPlanes />}
{parseErrors.length > 0 && (
<div
className={`absolute inset-0 rounded-lg p-2 ${
operationList.length &&
`bg-destroy-10/40 dark:bg-destroy-80/40`
className={`absolute inset-0 p-2 ${
operationList.length ||
(parseErrors.length > 0 &&
`bg-destroy-10/40 dark:bg-destroy-80/40`)
Comment on lines +164 to +167
Copy link
Contributor

Choose a reason for hiding this comment

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

The conditional logic for applying the background class needs correction. The current implementation with || followed by a string template literal will cause unexpected behavior - if operationList.length is truthy but not a string, it will be used as the class name instead of the intended background class.

Consider refactoring to use a ternary operator:

className={`absolute inset-0 p-2 ${
  (operationList.length || parseErrors.length > 0) 
    ? 'bg-destroy-10/40 dark:bg-destroy-80/40' 
    : ''
}`}

This ensures the background class is only applied when either condition is met, and always returns a string value.

Suggested change
className={`absolute inset-0 p-2 ${
operationList.length ||
(parseErrors.length > 0 &&
`bg-destroy-10/40 dark:bg-destroy-80/40`)
className={`absolute inset-0 p-2 ${
(operationList.length || parseErrors.length > 0)
? 'bg-destroy-10/40 dark:bg-destroy-80/40'
: ''
}`}

Spotted by Diamond

Is this helpful? React 👍 or 👎 to let us know.

}`}
>
<div className="text-sm bg-destroy-80 text-chalkboard-10 py-1 px-2 rounded flex gap-2 items-center">
<div className="text-base font-sans font-normal text-destroy-80 dark:text-destroy-10 bg-destroy-10 dark:bg-destroy-80 py-1 px-2 rounded flex gap-2 items-center">
<p className="flex-1">
Errors found in KCL code.
<br />
Please fix them before continuing.
</p>
<button
onClick={goToError}
className="bg-chalkboard-10 text-destroy-80 p-1 rounded-sm flex-none hover:bg-chalkboard-10 hover:border-destroy-70 hover:text-destroy-80 border-transparent"
className="border bg-chalkboard-10 text-destroy-80 p-1 rounded flex-none hover:bg-chalkboard-10 hover:border-destroy-70 hover:text-destroy-80 border-transparent"
>
View error
</button>
Expand Down
Loading