Skip to content

Commit 86fc9cb

Browse files
thatblindgeyethatblindgeye
committed
Feedback from Nicole
1 parent c45642b commit 86fc9cb

8 files changed

Lines changed: 50 additions & 25 deletions

File tree

‎packages/react-core/src/components/Button/Button.tsx‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ export interface ButtonProps extends Omit<React.HTMLProps<HTMLButtonElement>, 'r
111111
isCircle?: boolean;
112112
/** @beta Flag indicating the button is a docked variant button. For use in docked navigation. */
113113
isDocked?: boolean;
114-
/** @beta Flag indicating the dock button should display text. Only applies when isDock is true. */
114+
/** @beta Flag indicating the dock button should display text. Only applies when isDocked is true. */
115115
isTextExpanded?: boolean;
116116
/** @hide Forwarded ref */
117117
innerRef?: React.Ref<any>;
@@ -271,8 +271,8 @@ const ButtonBase: React.FunctionComponent<ButtonProps> = ({
271271
size === ButtonSize.sm && styles.modifiers.small,
272272
size === ButtonSize.lg && styles.modifiers.displayLg,
273273
isCircle && styles.modifiers.circle,
274-
isDocked && styles.modifiers.dock, // Replace wwith docked class from https://github.com/patternfly/patternfly/pull/8308
275-
isTextExpanded && styles.modifiers.textExpanded,
274+
isDocked && styles.modifiers.dock, // Replace with docked class from https://github.com/patternfly/patternfly/pull/8308
275+
isDocked && isTextExpanded && styles.modifiers.textExpanded,
276276
className
277277
)}
278278
disabled={isButtonElement ? isDisabled : null}

‎packages/react-core/src/components/Button/__tests__/Button.test.tsx‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -566,8 +566,12 @@ describe('Dock variant', () => {
566566
expect(screen.getByRole('button')).not.toHaveClass(styles.modifiers.dock);
567567
});
568568

569-
test(`Renders with class ${styles.modifiers.textExpanded} when isTextExpanded = true`, () => {
570-
render(<Button isTextExpanded>Text Expanded Button</Button>);
569+
test(`Renders with class ${styles.modifiers.textExpanded} when isTextExpanded = true and isDocked = true`, () => {
570+
render(
571+
<Button isTextExpanded isDocked>
572+
Text Expanded Button
573+
</Button>
574+
);
571575
expect(screen.getByRole('button')).toHaveClass(styles.modifiers.textExpanded);
572576
});
573577

@@ -576,6 +580,11 @@ describe('Dock variant', () => {
576580
expect(screen.getByRole('button')).not.toHaveClass(styles.modifiers.textExpanded);
577581
});
578582

583+
test(`Does not render with class ${styles.modifiers.textExpanded} when isTextExpanded = true but isDocked is not passed`, () => {
584+
render(<Button isTextExpanded>Text Expanded Button</Button>);
585+
expect(screen.getByRole('button')).not.toHaveClass(styles.modifiers.textExpanded);
586+
});
587+
579588
test(`Renders with both ${styles.modifiers.dock} and ${styles.modifiers.textExpanded} when both props are true`, () => {
580589
render(
581590
<Button isDocked isTextExpanded>

‎packages/react-core/src/components/Button/__tests__/__snapshots__/Button.test.tsx.snap‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ exports[`Renders basic button 1`] = `
55
<button
66
aria-label="basic button"
77
class="pf-v6-c-button pf-m-primary"
8-
data-ouia-component-id="OUIA-Generated-Button-primary-:r35:"
8+
data-ouia-component-id="OUIA-Generated-Button-primary-:r36:"
99
data-ouia-component-type="PF6/Button"
1010
data-ouia-safe="true"
1111
type="button"

‎packages/react-core/src/components/MenuToggle/MenuToggle.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -198,8 +198,8 @@ class MenuToggleBase extends Component<MenuToggleProps> {
198198
isDisabled && styles.modifiers.disabled,
199199
isPlaceholder && styles.modifiers.placeholder,
200200
isSettings && styles.modifiers.settings,
201-
isDocked && styles.modifiers.dock, // Replace wwith docked class from https://github.com/patternfly/patternfly/pull/8308
202-
isTextExpanded && styles.modifiers.textExpanded,
201+
isDocked && styles.modifiers.dock, // Replace with docked class from https://github.com/patternfly/patternfly/pull/8308
202+
isDocked && isTextExpanded && styles.modifiers.textExpanded,
203203
size === MenuToggleSize.sm && styles.modifiers.small,
204204
className
205205
);

‎packages/react-core/src/components/MenuToggle/__tests__/MenuToggle.test.tsx‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,8 +166,12 @@ test(`Does not render with class ${styles.modifiers.dock} when isDocked is not p
166166
expect(screen.getByRole('button')).not.toHaveClass(styles.modifiers.dock);
167167
});
168168

169-
test(`Renders with class ${styles.modifiers.textExpanded} when isTextExpanded is passed`, () => {
170-
render(<MenuToggle isTextExpanded>Text Expanded Toggle</MenuToggle>);
169+
test(`Renders with class ${styles.modifiers.textExpanded} when isTextExpanded is passed and isDocked is passed`, () => {
170+
render(
171+
<MenuToggle isTextExpanded isDocked>
172+
Text Expanded Toggle
173+
</MenuToggle>
174+
);
171175
expect(screen.getByRole('button')).toHaveClass(styles.modifiers.textExpanded);
172176
});
173177

@@ -176,6 +180,11 @@ test(`Does not render with class ${styles.modifiers.textExpanded} when isTextExp
176180
expect(screen.getByRole('button')).not.toHaveClass(styles.modifiers.textExpanded);
177181
});
178182

183+
test(`Does not render with class ${styles.modifiers.textExpanded} when isTextExpanded is passed but isDocked is not passed`, () => {
184+
render(<MenuToggle isTextExpanded>Text Expanded Toggle</MenuToggle>);
185+
expect(screen.getByRole('button')).not.toHaveClass(styles.modifiers.textExpanded);
186+
});
187+
179188
test(`Renders with both ${styles.modifiers.dock} and ${styles.modifiers.textExpanded} when both props are passed`, () => {
180189
render(
181190
<MenuToggle isDocked isTextExpanded>

‎packages/react-core/src/components/Nav/Nav.tsx‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ class Nav extends Component<NavProps, { isScrollable: boolean; flyoutRef: React.
125125
...props
126126
} = this.props;
127127
const isHorizontal = ['horizontal', 'horizontal-subnav'].includes(variant);
128-
128+
const isDocked = variant === 'docked';
129129
return (
130130
<SSRSafeIds prefix="pf-" ouiaComponentType={`Nav${variant ? `-${variant}` : ''}`}>
131131
{(_, generatedOuiaId) => (
@@ -157,9 +157,9 @@ class Nav extends Component<NavProps, { isScrollable: boolean; flyoutRef: React.
157157
className={css(
158158
styles.nav,
159159
isHorizontal && styles.modifiers.horizontal,
160-
variant === 'docked' && styles.modifiers.docked,
160+
isDocked && styles.modifiers.docked,
161161
variant === 'horizontal-subnav' && styles.modifiers.subnav,
162-
isTextExpanded && styles.modifiers.textExpanded,
162+
isDocked && isTextExpanded && styles.modifiers.textExpanded,
163163
this.state.isScrollable && styles.modifiers.scrollable,
164164
className
165165
)}

‎packages/react-core/src/components/Nav/__tests__/Nav.test.tsx‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -275,9 +275,9 @@ describe('Nav', () => {
275275
expect(screen.getByTestId('docked-nav')).toHaveClass(styles.modifiers.docked);
276276
});
277277

278-
test(`Renders with ${styles.modifiers.textExpanded} class when isTextExpanded is true`, () => {
278+
test(`Renders with ${styles.modifiers.textExpanded} class when isTextExpanded is true and variant is docked`, () => {
279279
renderNav(
280-
<Nav isTextExpanded data-testid="text-expanded-nav">
280+
<Nav isTextExpanded variant="docked" data-testid="text-expanded-nav">
281281
<NavList>
282282
{props.items.map((item) => (
283283
<NavItem to={item.to} key={item.to}>
@@ -304,4 +304,19 @@ describe('Nav', () => {
304304
);
305305
expect(screen.getByTestId('nav')).not.toHaveClass(styles.modifiers.textExpanded);
306306
});
307+
308+
test(`Does not render with ${styles.modifiers.textExpanded} class when isTextExpanded is true but variant is not docked`, () => {
309+
renderNav(
310+
<Nav isTextExpanded data-testid="nav">
311+
<NavList>
312+
{props.items.map((item) => (
313+
<NavItem to={item.to} key={item.to}>
314+
{item.label}
315+
</NavItem>
316+
))}
317+
</NavList>
318+
</Nav>
319+
);
320+
expect(screen.getByTestId('nav')).not.toHaveClass(styles.modifiers.textExpanded);
321+
});
307322
});

‎packages/react-core/src/demos/examples/Nav/NavDockedNav.tsx‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -203,11 +203,7 @@ export const NavDockedNav: React.FunctionComponent = () => {
203203
/>
204204
</MastheadToggle>
205205
<MastheadBrand>
206-
<MastheadLogo component={(props) => <a {...props} href="#" />}>
207-
{mobileTextLogo}
208-
{/* <Brand src={pfHorizontalLogo} alt="PatternFly" heights={{ default: '36px' }} className="show-light" /> */}
209-
{/* <Brand src={pfHorizontalLogoReverse} alt="PatternFly" heights={{ default: '36px' }} className="show-dark" /> */}
210-
</MastheadLogo>
206+
<MastheadLogo component={(props) => <a {...props} href="#" />}>{mobileTextLogo}</MastheadLogo>
211207
</MastheadBrand>
212208
</MastheadMain>
213209
<MastheadContent>
@@ -240,11 +236,7 @@ export const NavDockedNav: React.FunctionComponent = () => {
240236
<MastheadLogo component={(props) => <a {...props} href="#" />} isCompact>
241237
<Brand src={pfIconLogo} alt="PatternFly" heights={{ default: '37px' }} />
242238
</MastheadLogo>
243-
<MastheadLogo component={(props) => <a {...props} href="#" />}>
244-
{dockTextLogo}
245-
{/* <Brand src={pfHorizontalLogo} alt="PatternFly" heights={{ default: '37px' }} className="show-light" />
246-
<Brand src={pfHorizontalLogoReverse} alt="PatternFly" heights={{ default: '37px' }} className="show-dark" /> */}
247-
</MastheadLogo>
239+
<MastheadLogo component={(props) => <a {...props} href="#" />}>{dockTextLogo}</MastheadLogo>
248240
</MastheadBrand>
249241
</MastheadMain>
250242
<Divider />

0 commit comments

Comments
 (0)