-
Notifications
You must be signed in to change notification settings - Fork 80
feat(navigation, navigation-logo, navigation-user): Add navigation, navigation-logo & navigation-user components. #6873
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
Changes from 74 commits
8bdfbc3
b5906c1
c2b64e0
ad24c69
d91619b
991691c
bf16349
199f634
95d13b9
ffeb67e
db19d11
b2f3254
a97cb56
cc8224c
80966ae
40793f4
cf105fb
7a0769f
2fecbe5
25573e1
675dd86
109a141
1ec3c32
ef32860
cef8b07
b53d6ca
98fc70c
777ee50
ab36471
357fee2
245728b
d2d0216
4323093
d07aab6
8aed351
1f6e312
98f816f
784d6c8
230e102
a0d18b1
a1c3700
4ca74e1
31515b1
4255077
52be92c
fbfb464
1f98e24
9343a9e
13fe1c3
a731b90
6912e84
ad2eb87
27ddef7
db51915
6708a1c
8b22471
2f76ede
01ce5d0
266da9c
856933b
af705d6
86180ad
e30fa6f
82331d0
e424aea
f453574
042e763
27550ac
b563d75
92830f7
2c7dcd5
bfaf90e
6095842
bd29cf8
7b88d1a
680ab82
c920b5f
66da54a
58943da
58269bc
11f42a2
07d01cb
7e462f8
9b1ad85
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
# calcite-nav-menu-item | ||
# calcite-menu-item | ||
|
||
<!-- Auto Generated Below --> | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
# calcite-nav-menu | ||
# calcite-menu | ||
|
||
<!-- Auto Generated Below --> | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,74 @@ | ||
import { Component, Element, h, Host, Prop, VNode } from "@stencil/core"; | ||
import { CSS } from "./resources"; | ||
|
||
@Component({ | ||
tag: "calcite-navigation-logo", | ||
styleUrl: "navigation-logo.scss", | ||
shadow: { | ||
delegatesFocus: true | ||
} | ||
}) | ||
export class CalciteNavigationLogo { | ||
//-------------------------------------------------------------------------- | ||
// | ||
// Element | ||
// | ||
//-------------------------------------------------------------------------- | ||
@Element() el: HTMLCalciteNavigationLogoElement; | ||
|
||
//-------------------------------------------------------------------------- | ||
// | ||
// Public Properties | ||
// | ||
//-------------------------------------------------------------------------- | ||
/** When true, the component is highlighted. */ | ||
anveshmekala marked this conversation as resolved.
Show resolved
Hide resolved
|
||
@Prop({ reflect: true }) active: boolean; | ||
|
||
/** Specifies the URL destination of the component, which can be set as an absolute or relative path.*/ | ||
@Prop({ reflect: true }) href: string; | ||
|
||
/** Describes the appearance or function of the `thumbnail`. If no label is provided, context will not be provided to assistive technologies. */ | ||
@Prop() label: string; | ||
|
||
/** Specifies the subtext to display, such as an organization or application description. */ | ||
@Prop() subtext: string; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this maybe be named There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we can use description here considering we change the cc @jcfranco There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. changed to heading and description. Wondering if we should drop |
||
|
||
/** Specifies the text to display, such as a product name.*/ | ||
@Prop() text: string; | ||
|
||
/** When `true`, displays the `text` and `subText` contents. */ | ||
@Prop({ reflect: true }) textEnabled = false; | ||
|
||
/** Specifies the `src` to an image. */ | ||
@Prop() thumbnail: string; | ||
|
||
// -------------------------------------------------------------------------- | ||
anveshmekala marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// | ||
// Render Methods | ||
// | ||
// -------------------------------------------------------------------------- | ||
|
||
render(): VNode { | ||
return ( | ||
<Host> | ||
<a href={this.href} tabindex={0}> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need to set There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe you could do There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we add There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If that's the case, it should probably follow There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i don't think There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. when href is The only time VoiceOver recognizes logo as Tested with VoicOver and safari 16.4 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. synced-up with kitty today. Updates regarding navigation-logo a11y:
|
||
{this.thumbnail && <img alt={this.label || ""} src={this.thumbnail} />} | ||
{(this.text || this.subtext) && this.textEnabled && ( | ||
anveshmekala marked this conversation as resolved.
Show resolved
Hide resolved
|
||
<div class={CSS.textContainer}> | ||
{this.text && ( | ||
<span class={CSS.logoText} key={CSS.logoText}> | ||
{this.text} | ||
</span> | ||
)} | ||
{this.subtext && ( | ||
<span class={CSS.logoSubtext} key={CSS.logoSubtext}> | ||
{this.subtext} | ||
</span> | ||
)} | ||
</div> | ||
)} | ||
</a> | ||
</Host> | ||
); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
import { accessible, defaults, hidden, reflects, renders } from "../../tests/commonTests"; | ||
|
||
describe("calcite-navigation-logo", () => { | ||
describe("renders", () => { | ||
renders("calcite-navigation-logo", { display: "inline-flex" }); | ||
}); | ||
|
||
describe("honors hidden attribute", () => { | ||
hidden("calcite-navigation-logo"); | ||
}); | ||
|
||
describe("accessible", () => { | ||
accessible("calcite-navigation-logo"); | ||
}); | ||
|
||
anveshmekala marked this conversation as resolved.
Show resolved
Hide resolved
|
||
it("reflects", () => | ||
reflects("calcite-navigation-logo", [ | ||
{ | ||
propertyName: "active", | ||
value: "true" | ||
}, | ||
{ | ||
propertyName: "href", | ||
value: "#logo" | ||
}, | ||
{ | ||
propertyName: "textEnabled", | ||
value: "true" | ||
} | ||
])); | ||
|
||
it("defaults", async () => | ||
defaults("calcite-navigation-logo", [ | ||
{ | ||
propertyName: "textEnabled", | ||
defaultValue: false | ||
} | ||
])); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,70 @@ | ||
:host { | ||
@apply inline-flex outline-none; | ||
& a { | ||
@apply flex | ||
m-0 | ||
items-center | ||
justify-center | ||
cursor-pointer | ||
transition-default | ||
focus-base | ||
no-underline; | ||
border-block-end: 2px solid transparent; | ||
} | ||
& img { | ||
@apply flex h-7 m-0; | ||
} | ||
} | ||
|
||
a:hover, | ||
a:focus { | ||
@apply bg-foreground-2; | ||
} | ||
|
||
a:focus { | ||
@apply focus-inset; | ||
} | ||
|
||
a:active { | ||
@apply bg-foreground-3; | ||
} | ||
|
||
img { | ||
padding-inline: 1rem; | ||
} | ||
|
||
img ~ .text-container { | ||
@apply ps-0; | ||
} | ||
|
||
:host(:active) a { | ||
@apply text-color-1; | ||
} | ||
|
||
:host([active]) a { | ||
@apply text-color-1; | ||
border-color: var(--calcite-ui-brand); | ||
--calcite-ui-icon-color: var(--calcite-ui-brand); | ||
} | ||
|
||
.text-container { | ||
@apply flex | ||
flex-col | ||
truncate | ||
text-start; | ||
padding-inline: 1rem; | ||
} | ||
|
||
.logo-text { | ||
@apply text-0 | ||
ms-0 | ||
truncate | ||
text-color-1 | ||
font-medium; | ||
margin-block-start: 2px; | ||
} | ||
|
||
.logo-subtext { | ||
@apply text-color-2 truncate; | ||
font-size: var(--calcite-font-size--1); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
import { boolean, storyFilters } from "../../../.storybook/helpers"; | ||
import { placeholderImage } from "../../../.storybook/placeholderImage"; | ||
import readme from "./readme.md"; | ||
import { html } from "../../../support/formatting"; | ||
import { text } from "@storybook/addon-knobs"; | ||
|
||
export default { | ||
title: "Components/Navigation/Navigation Logo", | ||
parameters: { | ||
notes: readme | ||
}, | ||
...storyFilters() | ||
}; | ||
|
||
export const simple = (): string => | ||
html`<calcite-navigation-logo | ||
subtext="${text("subtext", "City of AcmeCo")}" | ||
text="${text("text", "ArcGIS Online")}" | ||
thumbnail="${placeholderImage({ width: 50, height: 50 })}" | ||
${boolean("active", false)} | ||
${boolean("text-enabled", true)} | ||
/>`; | ||
|
||
export const text_TestOnly = (): string => html`<calcite-navigation-logo text="ArcGIS Online" text-enabled />`; | ||
|
||
export const subtext_TestOnly = (): string => | ||
html`<calcite-navigation-logo | ||
subtext="City of AcmeCo" | ||
text-enabled | ||
thumbnail="${placeholderImage({ width: 50, height: 50 })}" | ||
/>`; | ||
|
||
export const thumbnail_TestOnly = (): string => | ||
html`<calcite-navigation-logo thumbnail="${placeholderImage({ width: 50, height: 50 })}" />`; | ||
|
||
export const textAndThumbnail_TestOnly = (): string => html`<calcite-navigation-logo | ||
text="ArcGIS Online" | ||
thumbnail="${placeholderImage({ width: 50, height: 50 })}" | ||
text-enabled | ||
/>`; | ||
|
||
export const subtextAndThumbnail_TestOnly = (): string => html`<calcite-navigation-logo | ||
subtext="City of AcmeCo" | ||
thumbnail="${placeholderImage({ width: 50, height: 50 })}" | ||
text-enabled | ||
/>`; | ||
|
||
export const All_TestOnly = (): string => html`<calcite-navigation-logo | ||
text="ArcGIS Online" | ||
subtext="City of AcmeCo" | ||
thumbnail="${placeholderImage({ width: 50, height: 50 })}" | ||
text-enabled | ||
/>`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is related to visual regression found while debugging navigation component.