feat(navigation, navigation-logo, navigation-user): Add navigation, navigation-logo & navigation-user components. #6873

anveshmekala · 2023-04-27T15:30:01Z

Related Issue: #6531

Summary

This PR adds the following components:

calcite-navigation
calcite-navigation-user
calcite-navigation-logo

Co-authored by @macandcheese
Extracted from #6829

calcite-navigation

Usage

Basic

<calcite-shell>
  <calcite-navigation slot="header">
    <calcite-chip-group slot="content-center">
      <calcite-chip>nav item 1</calcite-chip>
      <calcite-chip>nav item 2</calcite-chip>
      <calcite-chip>nav item 3</calcite-chip>
    </calcite-chip-group>
  </calcite-navigation>
</calcite-shell>

Properties

Property	Attribute	Description	Type	Default
`label` (required)	`label`	When `navigationAction` is `true`, specifies the label of the `calcite-action`.	`string`	`undefined`
`navigationAction`	`navigation-action`	When `true`, displays a `calcite-action` and emits a `calciteNavActionSelect` event on selection change.	`boolean`	`false`

Events

Event	Description	Type
`calciteNavigationActionSelect`	When `navigationAction` is true, emits when the displayed action selection changes.	`CustomEvent<void>`

Methods

`setFocus() => Promise<void>`

When navigation-action is true, sets focus on the component's action element.

Returns

Type: Promise<void>

CSS Custom Properties

Name	Description
`--calcite-navigation-background`	Specifies the background color of the component.
`--calcite-navigation-border-color`	Specifies the border color of the component.
`--calcite-navigation-width`	Specifies the width of the component's content area.

Dependencies

Depends on

calcite-action

calcite-navigation-logo

Usage

Basic

<calcite-navigation-logo active thumbnail="./_assets/images/esri-logo.svg"></calcite-navigation-user>

Properties

Property	Attribute	Description	Type	Default
`active`	`active`	When true, the component is highlighted.	`boolean`	`undefined`
`description`	`description`	A description for the component, which displays below the `heading`.	`string`	`undefined`
`heading`	`heading`	Specifies heading text for the component, such as a product or organization name.	`string`	`undefined`
`href`	`href`	Specifies the URL destination of the component, which can be set as an absolute or relative path.	`string`	`undefined`
`label`	`label`	Describes the appearance or function of the `thumbnail`. If no label is provided, context will not be provided to assistive technologies.	`string`	`undefined`
`rel`	`rel`	Defines the relationship between the `href` value and the current document.	`string`	`undefined`
`target`	`target`	Specifies where to open the linked document defined in the `href` property.	`string`	`undefined`
`thumbnail`	`thumbnail`	Specifies the `src` to an image.	`string`	`undefined`

Methods

`setFocus() => Promise<void>`

Sets focus on the component.

Returns

Type: Promise<void>

calcite-navigation-user

Usage

Basic

<calcite-navigation-user full-name="Jhon Doe" user-id="Jhon123" active></calcite-navigation-user>

Properties

Property	Attribute	Description	Type	Default
`active`	`active`	When true, the component is highlighted.	`boolean`	`undefined`
`fullName`	`full-name`	Specifies the full name of the user.	`string`	`undefined`
`label`	`label`	Describes the appearance of the avatar. If no label is provided, context will not be provided to assistive technologies.	`string`	`undefined`
`textDisabled`	`text-disabled`	When `true`, hides the `fullName` and `username` contents.	`boolean`	`false`
`thumbnail`	`thumbnail`	Specifies the`src` to an image (remember to add a token if the user is private).	`string`	`undefined`
`userId`	`user-id`	Specifies the unique id of the user.	`string`	`undefined`
`username`	`username`	Specifies the username of the user.	`string`	`undefined`

Methods

`setFocus() => Promise<void>`

Sets focus on the component.

Returns

Type: Promise<void>

Dependencies

Depends on

calcite-avatar

src/components/nav/nav.tsx

driskull

Initial review! LOoking good! 👀

src/components/nav-logo/nav-logo.tsx

src/components/nav/nav.tsx

stencil.config.ts

macandcheese · 2023-04-27T21:10:57Z

src/demos/_assets/images/dashboard-logo.svg

Is there a better way to use assets in demo pages? Maybe we can use url if these are publicly available to not include in project.

src/components/nav-logo/nav-logo.scss

src/components/nav/nav.tsx

…/github.com/Esri/calcite-components into anveshmekala/6531-feat-navigation-component

src/components/nav/nav.stories.ts

src/components/nav-logo/nav-logo.stories.ts

jcfranco

Awesome stuff, @anveshmekala!

Once the setFocus and navigation action prop/event renames are addressed, this should be good to merge!

src/components/navigation-logo/navgation-logo.tsx

jcfranco · 2023-05-19T17:54:38Z

src/components/navigation-logo/navgation-logo.tsx

+  render(): VNode {
+    return (
+      <Host>
+        <a href={this.href} tabindex={0}>


No need to set tabindex here as anchor's is 0 by default.

the tabIndex still needs to be set here because the default 0 is applied only when href has a valid value or an empty string. I tried adding empty string which results in reload of page. Later changed to "#" which affects a11y instructions.

Maybe you could do tabindex={!this.href ? 0 : null}. href should be a required prop though right?

tabindex={!this.href ? 0 : null} this will over-ride the default tabindex value 0 with null when href is defined and href is not a required prop.

Should we add rel & target prop here?

If that's the case, it should probably follow calcite-button logic and render either a button or a anchor depending on the existence of href.

i don't think href should be a required attribute for logo. Is there an added advantage of rendering a button when href is undefined besides click event being emitted on Space keydown ?

I think it would just be a11y concerns. An anchor should take you somewhere (either on the page or off) whereas as button should perform some action.

when href is undefined & text-enabled is false , VoiceOver is reads out the logo as {label} name, image.
when href is undefined & text-enabled is true , VoiceOver focuses the image reading the instruction as {label} name, image and then user can move on to the text which is read out.

The only time VoiceOver recognizes logo as link is when user adds href. When both href and label are undefined , the instructions recognizes logo as clickable element.

Tested with VoicOver and safari 16.4

synced-up with kitty today.

Updates regarding navigation-logo a11y:

tabindex default value is removed because logo shouldn't be in the focus order when href is null value.

AT user should be able to hear out image (if label is defined), heading & description context.

Screen readers will recognize logo as link if href is defined.

src/components/navigation-logo/navgation-logo.tsx

src/components/navigation-logo/navigation-logo.e2e.ts

src/components/navigation-user/usage/Basic.md

jcfranco · 2023-05-19T18:14:00Z

src/components/navigation/navigation.e2e.ts

+    expect(eventSpy).toHaveReceivedEventTimes(1);
+
+    await page.keyboard.press("Space");
+    await page.waitForChanges();


I don't think waitForChanges is needed here as it wasn't needed for Enter.

tests are failing without waitForChanges on Space key . I see that waitForChanges is added in many other instances on Space keydown.

src/components/navigation/navigation.tsx

src/utils/dom.ts

jcfranco

Awesome stuff, @anveshmekala!

Once the setFocus and navigation action prop/event renames are addressed, this should be good to merge!

driskull

Nice! 👍 💯

driskull · 2023-05-19T18:32:44Z

src/components/navigation-logo/navgation-logo.tsx

+  @Prop() label: string;
+
+  /** Specifies the subtext to display, such as an organization or application description. */
+  @Prop() subtext: string;


Should this maybe be named description to be consistent with other props?

we can use description here considering we change the text attribute to something like name or label.

cc @jcfranco

changed to heading and description. Wondering if we should drop textEnabled prop and render heading & description if provided by the user.

src/components/navigation-user/navigation-user.tsx

driskull · 2023-05-19T18:36:28Z

src/components/navigation-user/navigation-user.tsx

+    return (
+      <Host>
+        <button aria-label={this.label}>
+          <calcite-avatar


note: I wonder if it might make more sense to just have them slot the <calcite-avatar> instead of this component having all the same props?

The reason for having a separate component is to restrict the users from setting the scale which can cause layout issues in navigation context me thinks. We can considering re-using avatar component by rendering username & fullName in the DOM and adjusting scaling based on CSS selectors to identify if its slotted inside navigation component.

WDYT @macandcheese @jcfranco

We could always modify the scale of the avatar onslotchange.

feat(nav,nav-logo,nav-user): Add nav, nav-logo & nav-user components.

8bdfbc3

github-actions bot added the enhancement Issues tied to a new feature or request. label Apr 27, 2023

github-actions bot assigned anveshmekala Apr 27, 2023

anveshmekala added 2 commits April 27, 2023 14:44

add more 2e2 tests

b5906c1

rename prop in nav and add e2e test

c2b64e0

anveshmekala commented Apr 27, 2023

View reviewed changes

src/components/nav/nav.tsx Outdated Show resolved Hide resolved

anveshmekala added 4 commits April 27, 2023 15:17

add demo-dom-swapper wrapper in demo files

ad24c69

remove default decorator options

d91619b

Merge branch 'master' into anveshmekala/6531-feat-navigation-component

991691c

update readme files

bf16349

anveshmekala marked this pull request as ready for review April 27, 2023 20:29

anveshmekala requested a review from a team as a code owner April 27, 2023 20:29

anveshmekala added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Apr 27, 2023

driskull requested changes Apr 27, 2023

View reviewed changes

macandcheese reviewed Apr 27, 2023

View reviewed changes

driskull reviewed Apr 27, 2023

View reviewed changes

src/components/nav-logo/nav-logo.scss Outdated Show resolved Hide resolved

src/components/nav/nav.tsx Outdated Show resolved Hide resolved

macandcheese mentioned this pull request Apr 28, 2023

feat(nav, menu): Add Nav, Nav Logo, Nav User, Menu, Menu Item components #6829

Closed

macandcheese and others added 4 commits April 28, 2023 15:30

Update stories

199f634

Merge branch 'anveshmekala/6531-feat-navigation-component' of https:/…

95d13b9

…/github.com/Esri/calcite-components into anveshmekala/6531-feat-navigation-component

feedback changes

ffeb67e

rename menuAction prop and fix e2e tests

db19d11

anveshmekala added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Apr 29, 2023

macandcheese reviewed Apr 30, 2023

View reviewed changes

src/components/nav/nav.stories.ts Outdated Show resolved Hide resolved

anveshmekala added 5 commits May 1, 2023 11:49

remove async for common tests

b2f3254

test errors

a97cb56

restore stencil config

cc8224c

Merge branch 'master' into anveshmekala/6531-feat-navigation-component

80966ae

update stories

40793f4

anveshmekala commented May 1, 2023

View reviewed changes

src/components/nav-logo/nav-logo.stories.ts Outdated Show resolved Hide resolved

anveshmekala changed the title ~~feat(navigation, navigation-logo, navigation-user): Add nav, nav-logo & nav-user components.~~ feat(navigation, navigation-logo, navigation-user): Add navigation, navigation-logo & navigation-user components. May 19, 2023

Merge branch 'master' into anveshmekala/6531-feat-navigation-component

bd29cf8

anveshmekala added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels May 19, 2023

jcfranco approved these changes May 19, 2023

View reviewed changes

driskull approved these changes May 19, 2023

View reviewed changes

anveshmekala added 7 commits May 19, 2023 16:39

more feedback changes

7b88d1a

Merge branch 'master' into anveshmekala/6531-feat-navigation-component

680ab82

add rel & target props to logo

c920b5f

fix logo slot issue in navigation

66da54a

change text and subText to heading & desctiption in logo

58943da

Merge branch 'master' into anveshmekala/6531-feat-navigation-component

58269bc

remove textEnabled prop from nav-logo

11f42a2

anveshmekala added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels May 22, 2023

fix navigation screenshot test

07d01cb

anveshmekala added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels May 22, 2023

anveshmekala added 2 commits May 22, 2023 12:57

remove tabindex and focus logo only if href is provided

7e462f8

update readme files

9b1ad85

anveshmekala added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels May 22, 2023

anveshmekala merged commit 167f9f8 into master May 22, 2023

anveshmekala deleted the anveshmekala/6531-feat-navigation-component branch May 22, 2023 19:09

github-actions bot added this to the 2023 May Priorities milestone May 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(navigation, navigation-logo, navigation-user): Add navigation, navigation-logo & navigation-user components. #6873

feat(navigation, navigation-logo, navigation-user): Add navigation, navigation-logo & navigation-user components. #6873

anveshmekala commented Apr 27, 2023 •

edited

Loading

driskull left a comment

macandcheese Apr 27, 2023

jcfranco left a comment

jcfranco May 19, 2023

anveshmekala May 19, 2023 •

edited

Loading

driskull May 19, 2023

anveshmekala May 19, 2023

anveshmekala May 19, 2023

driskull May 20, 2023

anveshmekala May 20, 2023

driskull May 20, 2023

anveshmekala May 20, 2023

anveshmekala May 22, 2023 •

edited

Loading

jcfranco May 19, 2023

anveshmekala May 19, 2023

jcfranco left a comment

driskull left a comment

driskull May 19, 2023

anveshmekala May 19, 2023

anveshmekala May 22, 2023

driskull May 19, 2023

anveshmekala May 19, 2023

driskull May 19, 2023

feat(navigation, navigation-logo, navigation-user): Add navigation, navigation-logo & navigation-user components. #6873

feat(navigation, navigation-logo, navigation-user): Add navigation, navigation-logo & navigation-user components. #6873

Conversation

anveshmekala commented Apr 27, 2023 • edited Loading

Summary

calcite-navigation

Usage

Basic

Properties

Events

Methods

setFocus() => Promise<void>

Returns

CSS Custom Properties

Dependencies

Depends on

calcite-navigation-logo

Usage

Basic

Properties

Methods

setFocus() => Promise<void>

Returns

calcite-navigation-user

Usage

Basic

Properties

Methods

setFocus() => Promise<void>

Returns

Dependencies

Depends on

driskull left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jcfranco left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anveshmekala May 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anveshmekala May 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jcfranco left a comment

Choose a reason for hiding this comment

driskull left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anveshmekala commented Apr 27, 2023 •

edited

Loading

`setFocus() => Promise<void>`

`setFocus() => Promise<void>`

`setFocus() => Promise<void>`

anveshmekala May 19, 2023 •

edited

Loading

anveshmekala May 22, 2023 •

edited

Loading