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.

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

Jump to bottom

chore(chrome): update prop descriptions #954

Merged
merged 9 commits into from
Dec 17, 2020
Merged

chore(chrome): update prop descriptions #954

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
486a428
Update chrome component prop docs
Mikaelia Dec 15, 2020
12f2145
Doc updates
Mikaelia Dec 15, 2020
a7a3633
Remove redundant word
Mikaelia Dec 16, 2020
720e790
Add missing backticks
Mikaelia Dec 16, 2020
edede97
Complete missing docs
Mikaelia Dec 17, 2020
a513cbf
Add additional missing docs
Mikaelia Dec 17, 2020
8aeb233
Tweak header item prop descriptions
Mikaelia Dec 17, 2020
fc61c57
Update docs per PR comments
Mikaelia Dec 17, 2020
5b813ae
Add cross-reference link
jzempel Dec 17, 2020
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
4 changes: 2 additions & 2 deletions packages/chrome/src/elements/Chrome.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ import { ChromeContext } from '../utils/useChromeContext';
import { StyledChrome } from '../styled';

interface IChromeProps extends HTMLAttributes<HTMLDivElement> {
/** Apply a custom hue to chrome navigation */
/** Applies a custom hue to the chrome navigation */
hue?: string;
/** Prevent fixed positioning from being applied to the <html> element */
/** Prevents fixed positioning from being applied to the `<html>` element */
isFluid?: boolean;
}

Expand Down
4 changes: 1 addition & 3 deletions packages/chrome/src/elements/body/Body.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,7 @@ import { StyledBody } from '../../styled';
import { BodyContext } from '../../utils/useBodyContext';

interface IBodyProps {
/**
* Prepare the body content height to allow space for a footer component
**/
/** Adjusts the body content height to allow space for a footer component */
hasFooter?: boolean;
}

Expand Down
1 change: 1 addition & 0 deletions packages/chrome/src/elements/header/HeaderItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ interface IHeadItemProps
extends IStyledBaseHeaderItemProps,
IStyledLogoHeaderItemProps,
HTMLAttributes<HTMLElement> {
/** Applies header logo styles to the item */
hasLogo?: boolean;
}

Expand Down
4 changes: 1 addition & 3 deletions packages/chrome/src/elements/nav/Nav.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@ import { NavContext } from '../../utils/useNavContext';
import { StyledNav } from '../../styled';

interface INavProps extends HTMLAttributes<HTMLElement> {
/**
* Expand navigation area to include item text
**/
/** Expands the nav area to display the item text */
isExpanded?: boolean;
}

Expand Down
16 changes: 4 additions & 12 deletions packages/chrome/src/elements/nav/NavItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,13 @@ import { useNavContext } from '../../utils/useNavContext';
import { useChromeContext } from '../../utils/useChromeContext';

interface INavItemProps extends HTMLAttributes<any> {
/**
* Applies product-specific color palette
**/
/** Applies a product-specific color palette */
product?: PRODUCT;
/**
* Indicate which item is current in the nav
**/
/** Indicates that the item is current in the nav */
isCurrent?: boolean;
/**
* Indicate that the item contains a product logo
*/
/** Indicates that the item contains a product logo */
hasLogo?: boolean;
/**
* Indicate that the item contains the company brandmark
*/
/** Indicates that the item contains the company brandmark */
hasBrandmark?: boolean;
}

Expand Down
7 changes: 7 additions & 0 deletions packages/chrome/src/elements/subnav/CollapsibleSubNavItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,15 @@ import {

export interface ICollapsibleSubNavItemProps
extends Omit<ButtonHTMLAttributes<HTMLButtonElement>, 'onChange'> {
/** Sets the item header */
header?: React.ReactNode;
/** Reveals the item text */
Copy link
Member

Choose a reason for hiding this comment

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

Replace item text with something like "item's content" or "sub-items" (not sure that's a term we want to use 😝 ) or, similar to Accordion "item's section" (as this is technically an accordion control).

See the demo at https://garden.zendesk.com/components/chrome#navigation-panel with "show subnav" activated for additional working details for a CollapsibleSubNavItem.

isExpanded?: boolean;
/**
* Handles changes in the item's expansion state
*
* @param {boolean} isExpanded An item's expansion state
*/
onChange?: (isExpanded: boolean) => void;
}

Expand Down
1 change: 1 addition & 0 deletions packages/chrome/src/elements/subnav/SubNavItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { StyledSubNavItem } from '../../styled';
import { useChromeContext } from '../../utils/useChromeContext';

interface ISubNavItemProps {
/** Indicates that the item is current in the subnav */
isCurrent?: boolean;
}

Expand Down
12 changes: 3 additions & 9 deletions packages/chrome/src/styled/header/StyledBaseHeaderItem.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,11 @@ import {
const COMPONENT_ID = 'chrome.base_header_item';

export interface IStyledBaseHeaderItemProps {
/**
* Horizontally maximize a flex item in the header to take as much space as possible (i.e. breadcrumb container)
**/
/** Maximizes the width of a flex item in the header (i.e. breadcrumb container) */
Copy link
Member

Choose a reason for hiding this comment

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

We should remove (i.e. breadcrumb container) as design research has determined planting a breadcrumb in the header is off-pattern.

maxX?: boolean;
/**
* Vertically maximize the height for a header item (i.e. contains a search input)
**/
/** Maximizes the height of the item (i.e. contains a search input) */
maxY?: boolean;
/**
* Round the border radius for a header item (i.e. user icon)
**/
/** Rounds the border radius of the item (i.e. user icon) */
Copy link
Member

Choose a reason for hiding this comment

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

Per guidance from Content Design and https://garden.zendesk.com/components/avatar#how-to-use-it-well, I wonder if we should swap user with "person". Or does "person icon" just sound a bit weird for dev docs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we need the bit inside the parenthesis? For me I'm not sure if it adds value as it seems pretty self-explanatory

Copy link
Member

Choose a reason for hiding this comment

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

Nope, good call.

isRound?: boolean;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/chrome/src/styled/header/StyledHeader.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { getNavItemHeight } from '../nav/StyledBaseNavItem';
const COMPONENT_ID = 'chrome.header';

export interface IStyledHeaderProps {
/** Display logo for standlone usage */
/** Displays logo for standlone usage */
isStandalone?: boolean;
}

Expand Down
4 changes: 1 addition & 3 deletions packages/chrome/src/styled/header/StyledHeaderItemText.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,7 @@ import { retrieveComponentStyles, DEFAULT_THEME } from '@zendeskgarden/react-the
const COMPONENT_ID = 'chrome.header_item_text';

export interface IStyledHeaderItemTextProps {
/**
* Clip text (but leave accessible to screenreaders) for an icon-only header item
**/
/** Hides item text. Text remains accessible to screenreaders. */
Copy link
Member

Choose a reason for hiding this comment

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

screenreaders -> "screen readers". https://en.wikipedia.org/wiki/Screen_reader

isClipped?: boolean;
}

Expand Down
4 changes: 1 addition & 3 deletions packages/chrome/src/styled/header/StyledLogoHeaderItem.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@ import { getNavWidth } from '../nav/StyledNav';
const COMPONENT_ID = 'chrome.header_item';

export interface IStyledLogoHeaderItemProps {
/**
* Applies product-specific color palette
**/
/** Applies a product-specific color palette */
Copy link
Member

Choose a reason for hiding this comment

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

I think this could be improved with "Applies a brand color to the product logo".

For the fussy details, the given name is used to dereference a brand color from the product portion of the PALETTE object. And then that color hex is inherited by currentColor used within one of the two-tone "relationshape" icons seen here: https://garden.zendesk.com/design/icons/26.

Copy link
Member

Choose a reason for hiding this comment

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

TBH, I regret having built this component 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh, I see. Thanks for the explanation!

product?: PRODUCT;
}

Expand Down
5 changes: 2 additions & 3 deletions packages/chrome/src/styled/nav/StyledNavItemText.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,9 @@ import { getNavWidth } from './StyledNav';
const COMPONENT_ID = 'chrome.nav_item_text';

export interface IStyledNavItemTextProps {
/**
* Wrap overflow text instead of truncating long strings with an ellipsis
**/
/** Wraps overflow item text instead of truncating long strings with an ellipsis **/
isWrapped?: boolean;
/** Reveals item text */
isExpanded?: boolean;
}

Expand Down
4 changes: 2 additions & 2 deletions packages/chrome/src/styled/subnav/StyledSubNavItemText.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ const COMPONENT_ID = 'chrome.subnav_item_text';

export interface IStyledSubNavItemTextProps {
/**
* Wrap overflow text instead of truncating long strings with an ellipsis
* (use in conjunction with max-width styling applied to the `SubNav` container)
* Wraps overflow item text instead of truncating long strings with an ellipsis.
* Use when `max-width` styling is applied to the subnav container.
**/
isWrapped?: boolean;
}
Expand Down