Skip to content
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
48 changes: 14 additions & 34 deletions packages/react-core/src/components/Compass/Compass.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
import { Drawer, DrawerContent, DrawerContentBody, DrawerProps } from '../Drawer';
import styles from '@patternfly/react-styles/css/components/Compass/compass';
import { css } from '@patternfly/react-styles';

import compassBackgroundImageLight from '@patternfly/react-tokens/dist/esm/c_compass_BackgroundImage_light';
import compassBackgroundImageDark from '@patternfly/react-tokens/dist/esm/c_compass_BackgroundImage_dark';

export interface CompassProps extends React.HTMLProps<HTMLDivElement> {
/** Additional classes added to the Compass. */
className?: string;
Expand Down Expand Up @@ -40,10 +36,6 @@
drawerContent?: React.ReactNode;
/** Additional props passed to the drawer */
drawerProps?: DrawerProps;
/** Light theme background image path of the Compass */
backgroundSrcLight?: string;
/** Dark theme background image path of the Compass */
backgroundSrcDark?: string;
}

export const Compass: React.FunctionComponent<CompassProps> = ({
Expand All @@ -63,26 +55,12 @@
isFooterExpanded = true,
drawerContent,
drawerProps,
backgroundSrcLight,
backgroundSrcDark,
...props
}: CompassProps) => {
const hasDrawer = drawerContent !== undefined;

const backgroundImageStyles: { [key: string]: string } = {};
if (backgroundSrcLight) {
backgroundImageStyles[compassBackgroundImageLight.name] = `url(${backgroundSrcLight})`;
}
if (backgroundSrcDark) {
backgroundImageStyles[compassBackgroundImageDark.name] = `url(${backgroundSrcDark})`;
}

const compassContent = (
<div
className={css(styles.compass, dock !== undefined && styles.modifiers.docked, className)}
{...props}
style={{ ...props.style, ...backgroundImageStyles }}
>
<div className={css(styles.compassContainer)}>

Check failure on line 63 in packages/react-core/src/components/Compass/Compass.tsx

View workflow job for this annotation

GitHub Actions / Build, test & deploy

Property 'compassContainer' does not exist on type '{ button: "pf-v6-c-button"; compass: "pf-v6-c-compass"; compassContent: "pf-v6-c-compass__content"; compassDock: "pf-v6-c-compass__dock"; compassDockMain: "pf-v6-c-compass__dock-main"; ... 22 more ...; toolbarItem: "pf-v6-c-toolbar__item"; }'. Did you mean 'compassContent'?

Check failure on line 63 in packages/react-core/src/components/Compass/Compass.tsx

View workflow job for this annotation

GitHub Actions / Build

Property 'compassContainer' does not exist on type '{ button: "pf-v6-c-button"; compass: "pf-v6-c-compass"; compassContent: "pf-v6-c-compass__content"; compassDock: "pf-v6-c-compass__dock"; compassDockMain: "pf-v6-c-compass__dock-main"; ... 22 more ...; toolbarItem: "pf-v6-c-toolbar__item"; }'. Did you mean 'compassContent'?
{dock && masthead}
{dock && (
<div
Expand Down Expand Up @@ -131,17 +109,19 @@
</div>
);

if (hasDrawer) {
return (
<Drawer isPill {...drawerProps}>
<DrawerContent panelContent={drawerContent}>
<DrawerContentBody>{compassContent}</DrawerContentBody>
</DrawerContent>
</Drawer>
);
}

return compassContent;
return (
<div className={css(styles.compass, dock !== undefined && styles.modifiers.docked, className)} {...props}>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Align docked-class condition with dock rendering condition.

Line 113 uses dock !== undefined, but dock rendering uses truthiness (dock && ...). With dock={false} / dock={null}, no dock is rendered while docked styling is still applied.

Suggested fix
-    <div className={css(styles.compass, dock !== undefined && styles.modifiers.docked, className)} {...props}>
+    <div className={css(styles.compass, Boolean(dock) && styles.modifiers.docked, className)} {...props}>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<div className={css(styles.compass, dock !== undefined && styles.modifiers.docked, className)} {...props}>
<div className={css(styles.compass, Boolean(dock) && styles.modifiers.docked, className)} {...props}>
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/react-core/src/components/Compass/Compass.tsx` at line 113, The
Compass component applies the docked CSS when checking dock !== undefined which
mismatches the dock rendering that uses truthiness; update the className
expression in the Compass JSX (the line that calls css(styles.compass, ...)) to
use the same truthy check as rendering (e.g., replace dock !== undefined &&
styles.modifiers.docked with dock && styles.modifiers.docked or !!dock &&
styles.modifiers.docked) so that the docked style is only applied when the dock
is actually rendered.

{hasDrawer ? (
<Drawer isPill {...drawerProps}>
<DrawerContent panelContent={drawerContent}>
<DrawerContentBody>{compassContent}</DrawerContentBody>
</DrawerContent>
</Drawer>
) : (
compassContent
)}
</div>
);
};

Compass.displayName = 'Compass';
Original file line number Diff line number Diff line change
Expand Up @@ -99,27 +99,6 @@ test('Renders with drawer when drawerContent is provided', () => {
expect(screen.getByText('Drawer content')).toBeVisible();
});

test('Renders with light background image when backgroundSrcLight is provided', () => {
const backgroundSrc = 'light-bg.jpg';
render(<Compass backgroundSrcLight={backgroundSrc} data-testid="compass" />);
expect(screen.getByTestId('compass')).toHaveStyle(`--pf-v6-c-compass--BackgroundImage--light: url(${backgroundSrc})`);
});

test('Renders with dark background image when backgroundSrcDark is provided', () => {
const backgroundSrc = 'dark-bg.jpg';
render(<Compass backgroundSrcDark={backgroundSrc} data-testid="compass" />);
expect(screen.getByTestId('compass')).toHaveStyle(`--pf-v6-c-compass--BackgroundImage--dark: url(${backgroundSrc})`);
});

test('Renders with both light and dark background images when both are provided', () => {
const lightSrc = 'light-bg.jpg';
const darkSrc = 'dark-bg.jpg';
render(<Compass backgroundSrcLight={lightSrc} backgroundSrcDark={darkSrc} data-testid="compass" />);
const compassElement = screen.getByTestId('compass');
expect(compassElement).toHaveStyle(`--pf-v6-c-compass--BackgroundImage--light: url(${lightSrc})`);
expect(compassElement).toHaveStyle(`--pf-v6-c-compass--BackgroundImage--dark: url(${darkSrc})`);
});

test('Renders with additional props spread to the component', () => {
render(<Compass aria-label="Test label" data-testid="compass" />);
expect(screen.getByTestId('compass')).toHaveAccessibleName('Test label');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,38 +6,42 @@ exports[`Matches the snapshot with basic layout 1`] = `
class="pf-v6-c-compass"
>
<div
class="pf-v6-c-compass__header pf-m-expanded"
class=""
>
<div>
Header
<div
class="pf-v6-c-compass__header pf-m-expanded"
>
<div>
Header
</div>
</div>
</div>
<div
class="pf-v6-c-compass__sidebar pf-m-start pf-m-expanded"
>
<div>
Sidebar start
<div
class="pf-v6-c-compass__sidebar pf-m-start pf-m-expanded"
>
<div>
Sidebar start
</div>
</div>
</div>
<div
class="pf-v6-c-compass__main"
>
<div>
Main content
<div
class="pf-v6-c-compass__main"
>
<div>
Main content
</div>
</div>
</div>
<div
class="pf-v6-c-compass__sidebar pf-m-end pf-m-expanded"
>
<div>
Sidebar end
<div
class="pf-v6-c-compass__sidebar pf-m-end pf-m-expanded"
>
<div>
Sidebar end
</div>
</div>
</div>
<div
class="pf-v6-c-compass__footer pf-m-expanded"
>
<div>
Footer
<div
class="pf-v6-c-compass__footer pf-m-expanded"
>
<div>
Footer
</div>
</div>
</div>
</div>
Expand All @@ -47,60 +51,64 @@ exports[`Matches the snapshot with basic layout 1`] = `
exports[`Matches the snapshot with drawer 1`] = `
<DocumentFragment>
<div
class="pf-v6-c-drawer pf-m-expanded pf-m-pill"
class="pf-v6-c-compass"
>
<div
class="pf-v6-c-drawer__main"
class="pf-v6-c-drawer pf-m-expanded pf-m-pill"
>
<div
class="pf-v6-c-drawer__content"
class="pf-v6-c-drawer__main"
>
<div
class="pf-v6-c-drawer__body"
class="pf-v6-c-drawer__content"
>
<div
class="pf-v6-c-compass"
class="pf-v6-c-drawer__body"
>
<div
class="pf-v6-c-compass__header pf-m-expanded"
class=""
>
<div>
Header
<div
class="pf-v6-c-compass__header pf-m-expanded"
>
<div>
Header
</div>
</div>
</div>
<div
class="pf-v6-c-compass__sidebar pf-m-start pf-m-expanded"
>
<div>
Sidebar start
<div
class="pf-v6-c-compass__sidebar pf-m-start pf-m-expanded"
>
<div>
Sidebar start
</div>
</div>
</div>
<div
class="pf-v6-c-compass__main"
>
<div>
Main content
<div
class="pf-v6-c-compass__main"
>
<div>
Main content
</div>
</div>
</div>
<div
class="pf-v6-c-compass__sidebar pf-m-end pf-m-expanded"
>
<div>
Sidebar end
<div
class="pf-v6-c-compass__sidebar pf-m-end pf-m-expanded"
>
<div>
Sidebar end
</div>
</div>
</div>
<div
class="pf-v6-c-compass__footer pf-m-expanded"
>
<div>
Footer
<div
class="pf-v6-c-compass__footer pf-m-expanded"
>
<div>
Footer
</div>
</div>
</div>
</div>
</div>
</div>
<div>
Drawer content
<div>
Drawer content
</div>
</div>
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ In a basic Compass layout, content can be passed to the following props to popul
- `sidebarEnd`: Content rendered at the horizontal end of the page (by default, the right side).
- `footer`: Content rendered at the bottom of the page.

To customize the background image of the `<Compass>` and `<CompassHero>` components, you can use their respective `backgroundSrcLight` and `backgroundSrcDark` props. You can also add and customize a color gradient background for the `<CompassHero>` component by using the `gradientLight` and `gradientDark` props.
The background image of `<Compass>` is set at a global level alongside the theme. You can customize the background image of `<CompassHero>` by using its `backgroundSrcLight` and `backgroundSrcDark` props, or you may set a gradient using the `gradientLight` and `gradientDark` props.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Documentation references unsupported CompassHero background props

Line 39 currently documents backgroundSrcLight/backgroundSrcDark/gradientLight/gradientDark on <CompassHero>, which is misleading and conflicts with the objective to configure Compass background globally via CSS variable on html.

Suggested wording
-The background image of `<Compass>` is set at a global level alongside the theme. You can customize the background image of `<CompassHero>` by using its `backgroundSrcLight` and `backgroundSrcDark` props, or you may set a gradient using the `gradientLight` and `gradientDark` props.
+The background image of `<Compass>` is configured globally via theme CSS variables. For examples, set the background CSS variable on the `html` element’s `style` attribute.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
The background image of `<Compass>` is set at a global level alongside the theme. You can customize the background image of `<CompassHero>` by using its `backgroundSrcLight` and `backgroundSrcDark` props, or you may set a gradient using the `gradientLight` and `gradientDark` props.
The background image of `<Compass>` is configured globally via theme CSS variables. For example, set the background CSS variable on the `html` element's `style` attribute.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/react-core/src/components/Compass/examples/Compass.md` at line 39,
The docs incorrectly claim <CompassHero> supports
backgroundSrcLight/backgroundSrcDark/gradientLight/gradientDark props; update
the Compass.md content to remove references to those props and instead document
the supported approach: set the Compass background globally via the CSS variable
on html (or the theme-level background) and mention that <CompassHero> does not
accept those background props. Edit the paragraph referencing <Compass> and
<CompassHero> to state the correct configuration method and clarify that
customization is global via the html CSS variable rather than per-hero props.


```ts isBeta file="CompassBasic.tsx"

Expand Down
Loading