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.

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

updated header documentation. #1646

Merged
merged 30 commits into from
Nov 16, 2023
Merged

updated header documentation. #1646

merged 30 commits into from
Nov 16, 2023

Conversation

siddhantrawal
Copy link
Contributor

@siddhantrawal siddhantrawal commented Oct 19, 2023

Changes

Updated documentation for the Header component.

Testing

Docs

@siddhantrawal siddhantrawal requested review from a team as code owners October 19, 2023 20:58
@siddhantrawal siddhantrawal requested review from gretanausedaite and r100-stack and removed request for a team October 19, 2023 20:58
@siddhantrawal
Copy link
Contributor Author

I added the other examples that were present inside the storybook but the code is very similar. Do you guys think we need these examples or we can just have text explaining that you can make the header slim, add extra elements, and place content in the center?

If we decide to keep the examples, how can I display them properly so it is not hidden (Like the text inside the header). Something like this:
image

@siddhantrawal siddhantrawal requested review from mayank99 and removed request for gretanausedaite October 20, 2023 19:08
@siddhantrawal siddhantrawal self-assigned this Oct 23, 2023
Copy link
Contributor

@mayank99 mayank99 left a comment

Choose a reason for hiding this comment

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

a lot of this page needs to be rewritten and restructured.

all the "CONNECT Portal" parts can be removed, our header is not applicable there at all. we should focus on what our header actually does.

@siddhantrawal
Copy link
Contributor Author

siddhantrawal commented Oct 24, 2023

a lot of this page needs to be rewritten and restructured.

all the "CONNECT Portal" parts can be removed, our header is not applicable there at all. we should focus on what our header actually does.

Updated. Any other suggestions on what can be added/removed?

apps/website/src/pages/docs/header.mdx Outdated Show resolved Hide resolved
apps/website/src/pages/docs/header.mdx Outdated Show resolved Hide resolved
Copy link
Contributor

@mayank99 mayank99 left a comment

Choose a reason for hiding this comment

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

the page is still missing meaningful documentation about how to actually use the component. i'm referring to things like explaining the props (appLogo/breadcrumbs/actions) and the subcomponents (HeaderLogo/HeaderButton)

apps/website/src/pages/docs/header.mdx Outdated Show resolved Hide resolved
apps/website/src/pages/docs/header.mdx Outdated Show resolved Hide resolved
apps/website/src/pages/docs/header.mdx Outdated Show resolved Hide resolved
apps/website/src/pages/docs/header.mdx Outdated Show resolved Hide resolved
@siddhantrawal
Copy link
Contributor Author

the page is still missing meaningful documentation about how to actually use the component. i'm referring to things like explaining the props (appLogo/breadcrumbs/actions) and the subcomponents (HeaderLogo/HeaderButton)

Added usage section.

apps/website/src/pages/docs/header.mdx Outdated Show resolved Hide resolved
apps/website/src/pages/docs/header.mdx Outdated Show resolved Hide resolved
apps/website/src/pages/docs/header.mdx Outdated Show resolved Hide resolved
apps/website/src/pages/docs/header.mdx Outdated Show resolved Hide resolved
apps/website/src/pages/docs/header.mdx Outdated Show resolved Hide resolved
examples/Header.full.tsx Outdated Show resolved Hide resolved
examples/Header.full.tsx Outdated Show resolved Hide resolved
examples/Header.full.tsx Outdated Show resolved Hide resolved
examples/Header.full.tsx Show resolved Hide resolved
examples/Header.full.tsx Outdated Show resolved Hide resolved
examples/Header.full.tsx Outdated Show resolved Hide resolved
apps/website/src/pages/docs/header.mdx Outdated Show resolved Hide resolved
apps/website/src/pages/docs/header.mdx Outdated Show resolved Hide resolved
examples/Header.full.tsx Show resolved Hide resolved
apps/website/src/pages/docs/header.mdx Outdated Show resolved Hide resolved
apps/website/src/pages/docs/header.mdx Outdated Show resolved Hide resolved
apps/website/src/pages/docs/header.mdx Outdated Show resolved Hide resolved
apps/website/src/pages/docs/header.mdx Outdated Show resolved Hide resolved
apps/website/src/pages/docs/header.mdx Outdated Show resolved Hide resolved
apps/website/src/pages/docs/header.mdx Outdated Show resolved Hide resolved
apps/website/src/pages/docs/header.mdx Outdated Show resolved Hide resolved
- Help takes you to Bentley Communities for Bentley Cloud and Web Services.
- User profile is a drop down with options for your user profile.
- Logo can be added using the `appLogo` prop and specifying the desired logo under the `<HeaderLogo>` subcomponent.
- Breadcrumbs trail can be implemented using the `breadcrumbs` prop and `<HeaderBreadcrumbs>` subcomponent. Individual buttons can be added within the breadcrumbs trail using the `<HeaderButton>` subcomponent. For a dropdown menu, the `menuItems` prop can be used inside the `<HeaderButton>` for showing items and `onClick()` property for handling actions.
Copy link
Member

Choose a reason for hiding this comment

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

onClick may not need to be mentioned as that can likely be figured out since HeaderButton is a Button

Copy link
Contributor

Choose a reason for hiding this comment

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

better yet, remove the last sentence and just link to the HeaderButton section, which describes everything in detail

Suggested change
- Breadcrumbs trail can be implemented using the `breadcrumbs` prop and `<HeaderBreadcrumbs>` subcomponent. Individual buttons can be added within the breadcrumbs trail using the `<HeaderButton>` subcomponent. For a dropdown menu, the `menuItems` prop can be used inside the `<HeaderButton>` for showing items and `onClick()` property for handling actions.
- Breadcrumbs trail can be implemented using the `breadcrumbs` prop and `<HeaderBreadcrumbs>` subcomponent. Individual buttons can be added within the breadcrumbs trail using the [`<HeaderButton>` subcomponent](#headerbutton-subcomponent).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

- Settings

### Actions
<LiveExample src='Header.full.tsx'>
Copy link
Member

@r100-stack r100-stack Nov 3, 2023

Choose a reason for hiding this comment

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

Since we are mentioning that <HeaderButton> can be made a link (above paragraph), would be good to show it in the example too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added, was not sure about what link to add for href, so passing empty string. Is that okay?

Copy link
Contributor

@mayank99 mayank99 left a comment

Choose a reason for hiding this comment

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

looks good for the most part, just a couple more minor comments

apps/website/src/pages/docs/header.mdx Outdated Show resolved Hide resolved
apps/website/src/pages/docs/header.mdx Outdated Show resolved Hide resolved
examples/Header.main.tsx Outdated Show resolved Hide resolved
examples/Header.full.tsx Outdated Show resolved Hide resolved
@siddhantrawal siddhantrawal merged commit 01104ac into main Nov 16, 2023
14 checks passed
@siddhantrawal siddhantrawal deleted the siddhant/update_docs_header branch November 16, 2023 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants