feat(pf4-nav): add nav component #626

jschuler · 2018-09-18T03:09:52Z

affects: @patternfly/react-core @patternfly/react-docs

ISSUES CLOSED: #547

Adds the nav component to pf4 react

karelhala

Great PR, I think that this component will be needed by many projects. We should really focus on correct delivery of this component.

karelhala · 2018-09-19T14:23:13Z

packages/patternfly-4/react-core/src/components/Nav/Nav.js

+
+  shouldComponentUpdate(nextProps, nextState) {
+    const { activeItem, activeGroup } = this.state;
+    if (activeItem === nextState.activeItem && activeGroup === nextState.activeGroup) {


Do I understand it correctly, that when consumers want to update the nav items they can only update if some item is active or not? What if there are some conditional items which will be shown only after some event happens? If so, is this correct behavior?

Removing this

karelhala · 2018-09-19T14:26:30Z

packages/patternfly-4/react-core/src/components/Nav/Nav.js

+
+  // Called back from NavListItem
+  updateActive(groupId, itemId) {
+    this.setState({


I don't think we should hold which group and item is active, but rather send these information to consumer so he can react on these events, set corresponding active item and this component will be re-rendered.

With this approach the navigation will be activated no matter what. But what if consumer wants to ignore such thing (maybe there is form which needs to be filled before changing the navigation, or consumer has to check if user has access to such nav item)

karelhala · 2018-09-19T14:30:55Z

packages/patternfly-4/react-core/src/components/Nav/Nav.js

+    const { children, className, ariaLabel, ...props } = this.props;
+
+    // Init the group and item states if unknown
+    let { activeItem, activeGroup } = this.state;


I don't think that shis should be our responsibility. If user forgets to set activeGroup and activeItem via props he should either be notified about it or nothing special should happen. But let's not do something magical like selecting first group and first item.

For instance app navigation should not be affected if user enters config, such config is still part of application, but is not present in app navigation. These applications exists and we should have them in mind as well.

karelhala · 2018-09-19T14:37:52Z

packages/patternfly-4/react-core/src/components/Nav/NavListItem.js

+    const reactElement = React.isValidElement(children);
+    let clonedChild;
+    if (reactElement) {
+      // This could be a react-router Link for example


onUpdate function should handle all these things and you shouldn't have to clone the children and such.

I need to clone here to attach the update function to a react element

karelhala · 2018-09-19T14:47:18Z

I might be getting the purpose of of-react components wrongly, so feel free to correct me. But I got an impression that we should just provide components which are as simple as possible. But I don't think this fits the equation.

Few important notes, do not hold the state of active items/groups rather notify consumer on what was clicked and let him decide what to do with such information.

Toggling is fine I guess, but notify the consumer about such event as well, there might be some items which are fetched from server so the RBAC is checked everytime user is expanding/collapsing group.

Also we shouldn't try to guess which items should be active as default since there might be parts of application which are not accessible trough app nav, but trough other parts of application and user would be really confused if first item would be active.

Don't get me wrong @jschuler this PR is awesome! I just think that this is the first component which consumers would use so make it as usable as possible. If you have any questions, just ping me on slack and we can talk about more examples and why I think they are important.

jschuler · 2018-09-19T17:31:47Z

Thanks for the great feedback @karelhala, i think you're right i will provide callback functions to user so they can decide what to do once an item is clicked and how to init the nav. Will modify the code to be less stateful and examples to show that the interaction decisions are driven from consumer side.

tlabaj · 2018-09-19T18:05:43Z

packages/patternfly-4/react-core/src/components/Nav/Nav.js

+  /** Additional classes added to the Nav */
+  className: PropTypes.string,
+  /** Non-visible label to describe the nav */
+  ariaLabel: PropTypes.string


I think we want to at least warn the consumer that an aria label is required here. Maybe a function similar to TextInput.

Also, we have been using aria-label for the prop name in other components

I'll remove this prop and let the user pass in their own aria-label if they choose.
@jgiardino should aria-label be required for the nav? didn't see it mentioned in the core nav docs.

jschuler · 2018-09-19T18:31:47Z

Build: http://pf-nav.surge.sh/components/nav
Will manually update this url when i make the code changes
@jgiardino

tlabaj · 2018-09-19T18:39:11Z

packages/patternfly-4/react-core/src/components/Nav/NavGroup.js

+const propTypes = {
+  id: PropTypes.string.isRequired,
+  title: PropTypes.string.isRequired,
+  ariaLabel: PropTypes.string.isRequired,


should be aria-label.
I have the same question here about the prop not being required. Should we default to something meaningful, or generate a warning?

I think we should generate a warning here for aria-label

I actually think that ariaLabel can be removed here . In the design it looks like it is only using aria-labelledby .

tlabaj · 2018-09-19T18:45:57Z

packages/patternfly-4/react-core/src/components/Nav/NavGroup.js

+};
+
+const NavGroup = ({
+  id,


It seems the purpose for the id is for the 'aria-labeledby' attribute. If that is the case we probably don't want to force the consumer to provide one. We can generate the id similar to how it is don in the Modal Component.

NavGroup is an internal component that is not exposed to consumer. It is used by NavList and passed all the props. id is not required and default is generated as you've mentioned

Yep. I see it now. Looks good!

tlabaj · 2018-09-19T18:49:57Z

packages/patternfly-4/react-core/src/components/Nav/NavGroup.js

+          >
+            <a
+              className={css(styles.navLink)}
+              id={ariaLabel ? '' : id}


I don't understand why we would want to assign the ariaLabel to the id here?

Depending on if aria-label is passed in, it will be used as text in an invisible <h2> section below and this id will not be added to the dom. If a custom aria-label is not passed in, then the id is set here and referenced for aria-labelledby

amarie401 · 2018-09-19T18:49:41Z

packages/patternfly-4/react-core/src/components/Nav/Nav.js

+  /** Additional classes added to the Nav */
+  className: PropTypes.string,
+  /** Non-visible label to describe the nav */
+  ariaLabel: PropTypes.string


should be aria-label here as we do with the other components

tlabaj · 2018-09-19T18:53:39Z

packages/patternfly-4/react-core/src/components/Nav/NavList.js

+  /** Title shown for sections or expandable lists */
+  title: PropTypes.string,
+  /** Non-visible label to describe the expandable list */
+  ariaLabel: PropTypes.string,


aria-label here.

tlabaj · 2018-09-19T18:58:27Z

packages/patternfly-4/react-core/src/components/Nav/examples/NavReactRouter.js

@@ -0,0 +1,44 @@
+import React from 'react';


can you remove the example. it is commented out.

jgiardino · 2018-09-19T20:38:29Z

Hey @jschuler, great job on this! There are just a couple of things I noticed:

aria-current="page" should only be applied to the link with class pf-m-current
In the Grouped Nav example, the html structure doesn't match the core example. The core example has the <ul> as a child of the <section> for a group. Where as the react implementation has the <section> as a child to the <ul>.
In the expandable nav examples, the html structure also doesn't match what's in core. Each first-level list item should be a child of the same <ul>. But instead there are separate <ul> elements for each first-level list item.
In the Expandable Nav (w/subnav titles) example, when I place focus on the hidden <h2> element, it announces that it's clickable, but I can't find anything in the html that would indicate that it's clickable. Do you happen to know why JAWS thinks these headers are clickable?

jgiardino · 2018-09-19T20:45:27Z

One more thing...

Please remove aria-hidden="false" from the <section> elements. According to wai-aria:

At the time of this writing, aria-hidden="false" is known to work inconsistently in browsers. As future implementations improve, use caution and test thoroughly before relying on this approach.

We don't need it, so let's just remove it.

jgiardino · 2018-09-20T13:12:32Z

Hey @jschuler,
I just opened an issue to update a couple of attributes in the pf-core example of nav: patternfly/patternfly#746

Do you think it would be possible to go ahead and include those updates in this PR, too?

mattnolting · 2018-09-20T19:11:03Z

Great work @jschuler! Here's what I found.

All `<nav>` elements

<nav class="pf-c-nav" aria-label> - aria-labels are missing values
I think our active and current states are mixed up (my fault). It shouldn't affect your implementation, but would rather be a css update in core.
- Hover state is correct.
- Active state should not change font-weight, color, or border.
- Current state is correct.
- @kybaker ^^ is this consistent with what you expect to see? http://pf-nav.surge.sh/components/nav/

Expandable nav

looks like <section class="pf-c-nav__subnav"> is being removed from the DOM. We need to keep that in the DOM, toggle class pf-m-expanded, then after animation has finished add hidden attr. There will definitely be more conversation about how we animate, but for now we can rely on css.
- on collapse - remove pf-m-expanded, setTimout() to ~ 550ms and add hidden
- on expand - remove hidden, setTimout() to ~ 20ms and add pf-m-expanded (allows animation to happen)

As mentioned by @jgiardino

Grouped nav

<nav>
  <section>
    <h2>
    <ul>
      <li>
        <a>

Expandable nav

<nav>
  <ul>
    <li>
      <a>
      <section>
        <ul>
          <li>

jschuler · 2018-09-25T16:56:14Z

@jgiardino @mattnolting

In the Expandable Nav (w/subnav titles) example, when I place focus on the hidden
element, it announces that it's clickable, but I can't find anything in the html that would indicate that it's clickable. Do you happen to know why JAWS thinks these headers are clickable?

I image the same issue would be present in the core version. I have not looked further into it. Maybe we can make this a followup issue?

jschuler · 2018-09-25T17:03:46Z

@mattnolting
Do you mind if we create a followup issue to track the animation for the expandable nav?

mattnolting · 2018-09-25T17:39:51Z

@jschuler ok by me!

coveralls · 2018-09-25T17:53:58Z

Pull Request Test Coverage Report for Build 2225

82 of 89 (92.13%) changed or added relevant lines in 6 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.3%) to 79.435%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
packages/patternfly-4/react-core/src/components/Nav/NavList.js	19	20	95.0%
packages/patternfly-4/react-core/src/components/Nav/Nav.js	28	34	82.35%

Totals
Change from base Build 2210:	0.3%
Covered Lines:	3132
Relevant Lines:	3683

💛 - Coveralls

jschuler · 2018-09-25T18:07:33Z

Updated output at: http://pf-nav.surge.sh/components/nav

amarie401 · 2018-09-25T20:10:32Z

packages/patternfly-4/react-core/src/components/Avatar/index.d.ts

@@ -0,0 +1 @@
+export { default as Avatar, AvatarProps } from './Avatar';


thanks for adding this

amarie401 · 2018-09-25T20:11:03Z

packages/patternfly-4/react-core/src/components/TextArea/TextArea.d.ts

+import { SFC, HTMLProps, FormEvent } from 'react';
+import { Omit } from '../../typeUtils';
+
+export interface TextAreaProps extends Omit<HTMLProps<HTMLInputElement>, 'onChange'> {


thanks for adding this!

tlabaj · 2018-09-25T18:55:08Z

packages/patternfly-4/react-core/src/components/Nav/Nav.d.ts

+  variant?: OneOf<typeof NavVariants, keyof typeof NavVariants>;
+  children?: ReactNode;
+  className?: string;
+  ariaLabel?: string;


I don't see a ariaLabel prop on the Nav.js
I think you should have an aria-Label prop according to the Accessibility guidance in the docs.

I am not a TypeScript expert, but are you missing onChange as a prop here?

aria-prop can be added to the Nav component if the user wishes to since i add the ...props to it. Don't think it has to be required?

tlabaj · 2018-09-25T19:54:28Z

packages/patternfly-4/react-core/src/components/Nav/NavList.js

+  /** True to make the list expandable */
+  expandable: PropTypes.bool,
+  /** Flag indicating whether the list is expanded */
+  expanded: PropTypes.bool,


I think for consistency we should rename this to isExpanded.

Ok, although personally I must say I am not a fan ;)

I personally have no issues with it :) . I did however get dinged on a review for similar names due to wanting consistency.

tlabaj · 2018-09-25T19:54:53Z

packages/patternfly-4/react-core/src/components/Nav/NavList.js

+  innerClassName: PropTypes.string,
+  /** Title shown for sections or expandable lists */
+  title: PropTypes.string,
+  /** True to make the list expandable */


I think for consistency we should rename this to isExpandable.

tlabaj · 2018-09-25T19:55:24Z

packages/patternfly-4/react-core/src/components/Nav/NavList.js

+  /** Flag indicating whether the list is expanded */
+  expanded: PropTypes.bool,
+  /** Flag indicating whether the list is active */
+  active: PropTypes.bool


should be isActive for consistency.

tlabaj · 2018-09-25T20:08:41Z

packages/patternfly-4/react-core/src/components/Nav/NavGroup.js

+const propTypes = {
+  id: PropTypes.string.isRequired,
+  title: PropTypes.string.isRequired,
+  ariaLabel: PropTypes.string.isRequired,


I actually think that ariaLabel can be removed here . In the design it looks like it is only using aria-labelledby .

tlabaj · 2018-09-25T20:08:59Z

packages/patternfly-4/react-core/src/components/Nav/NavGroup.js

+          >
+            <a
+              className={css(styles.navLink)}
+              id={ariaLabel ? null : id}


can you just set this to id

ariaLabel is used in the <h2> below. And this id attribute should only be added to the DOM if ariaLabel is not set. If ariaLabel is not set, aria-labelledby gets the value from this <a> tag. If ariaLabel is set, aria-labelledby gets the value from the <h2> tag.

I think it is the naming that is throwing me off. Maybe rename it to sectionTitle since that is what it really is. ?

the h2 section is not visible and only there for screen readers. The <section> always has aria-labelledby with an id either from the <a> tag or from the <h2>.

Also, this is always set since the id is generated so the connection for screen readers will always be there.

I also talked to Jenn, other suggestions are srText or srHeading for the prop name since this is technically not an aria-label.

Sounds good, I can rename it to srText

tlabaj · 2018-09-25T20:10:53Z

packages/patternfly-4/react-core/src/components/Nav/NavGroup.js

+  children: PropTypes.node.isRequired,
+  innerClassName: PropTypes.string.isRequired,
+  groupId: PropTypes.number.isRequired,
+  active: PropTypes.bool.isRequired


rename props for consistency
expandable -> isExpandable
active -> isActive
exapanded -> isExpanded

tlabaj · 2018-09-25T20:14:59Z

packages/patternfly-4/react-core/src/components/Nav/NavListItem.js

+      e.stopPropagation();
+    }
+    // Callback to Nav
+    updateActive(groupId, id);


should we add updateActive to the props?

No since it is only exposed on the Nav component

patternfly-build · 2018-09-26T13:23:22Z

PatternFly documentation deployment: https://626-pr-patternfly-react-patternfly.surge.sh

karelhala

Looks really great! One major concern about strictly setting NavList for groupped navigation. Plus perhaps we could improve the overall render logic with react contexts, but that is optional.

karelhala · 2018-09-26T14:37:51Z

packages/patternfly-4/react-core/src/components/Nav/Nav.js

+  render() {
+    const { variant, children, className, ...props } = this.props;
+
+    const getVariantStyle = type =>


Optional: I love this object mapping! It can be simplified, that you don't really have to call function.

const variantStyle = { [NavVariants.default]: styles.navList, [NavVariants.simple]: styles.navSimpleList, [NavVariants.horizontal]: styles.navHorizontalList, [NavVariants.tertiary]: styles.navTertiaryList }

And usage would be variantStyle[style] instead of getVariantStyle(type).

karelhala · 2018-09-26T14:54:08Z

packages/patternfly-4/react-core/src/components/Nav/Nav.js

+    let variantStyle;
+    // Figure out if we have groups
+    let isGroup = false;
+    if (Array.isArray(children)) {


I don't know if I understand this correctly, but if you are checking for groups and apply navList style for any navigation with groups this should be handled by default props.

If consumer wants to render tertiary nav with groups we should allow him to do it, he probably knows what he's doing and wants to do it anyways.

I'd remove this if statemenet altogether and just apply styles based on nav variant.

Removing the variant style logic from the conditional. Groups do not have a variant style applied to them anyways so even if the user passes it in it won't have an effect. I still need to check if the user has passed in groups tho since the html structure is different for those.

karelhala · 2018-09-26T15:00:22Z

packages/patternfly-4/react-core/src/components/Nav/Nav.js

+
+    let innerNav;
+    if (isGroup) {
+      innerNav = renderLists(


This copying children should be avoided, I used it in dropdown as well and am planing to remove it. It's not wrong, it just makes the rendering login a bit harder and it takes more time to render. To pass down parent's onToggle function you could use react-context so only parent Nav component will have onToggle function and child components can trigger such function as well. Here is great example https://hackernoon.com/how-to-use-the-new-react-context-api-fce011e7d87

I added context for the callback functions, but I still need to clone children to add indices.

You might want to check out https://reactjs.org/docs/react-api.html#reactchildren children structure should not be assumed. You will also want to us the isValidElement check if you plan to dive into props. It is possible a user will return null, undefined, false, stings, numbers, or functions all of which will throw on the property access.

karelhala · 2018-09-26T15:03:52Z

packages/patternfly-4/react-core/src/components/Nav/NavList.js

+  isActive: false
+};
+
+const renderChildren = (children, groupId, onSelect) =>


If you're gonna use react context you don't have to copy the children once again and just call onSelect from children right away.

Not sure how to add index to the children without cloning? I did add context for the onSelect tho.

karelhala · 2018-09-26T15:05:18Z

packages/patternfly-4/react-core/src/components/Nav/NavListItem.js

+    const defaultLink = (
+      <a
+        href={to}
+        onClick={e => this.onUpdate(e)}


Can be simplified to

onClick={this.onUpdate}

karelhala · 2018-09-26T15:06:15Z

packages/patternfly-4/react-core/src/components/Nav/NavListItem.js

+class NavListItem extends React.Component {
+  onUpdate(e) {
+    const { onSelect, groupId, id } = this.props;
+    if (e) {


No need to check for the event to be present, but if you really want to be defensive this can be simpliified with shortcircuit

e && e.stopPropagation();

karelhala · 2018-09-26T15:06:48Z

packages/patternfly-4/react-core/src/components/Nav/NavListItem.js

+      e.stopPropagation();
+    }
+    // Callback to Nav
+    onSelect(groupId, id, e);


Usually if you pass down the event it should be as first parameter.

karelhala · 2018-09-26T15:16:14Z

packages/patternfly-4/react-core/src/components/Nav/NavToggle.js

+  };
+
+  handleToggle = e => {
+    this.setState(({ value }) => ({


Optional: No need to use the function gimmick of setState, because you do not call it more than once.

const { value } = this.state; const { groupId, onToggle } = this.props; this.setState({ value: !value; }); onToggle && onToggle(e, groupId, !value);

The function call in setState might confuse someone, with this destructing of state and props you are mutating the state at the very end of function executin. But that's just my preference.

karelhala · 2018-09-26T15:16:54Z

packages/patternfly-4/react-core/src/components/Nav/NavVariants.d.ts

+  simple: 'simple';
+  horizontal: 'horizontal';
+  tertiary: 'tertiary';
+};


Missing EOL.

tlabaj · 2018-09-26T15:18:55Z

packages/patternfly-4/react-core/src/components/Nav/NavGroup.js

+          >
+            <a
+              className={css(styles.navLink)}
+              id={ariaLabel ? null : id}


I also talked to Jenn, other suggestions are srText or srHeading for the prop name since this is technically not an aria-label.

tlabaj · 2018-09-26T15:24:13Z

packages/patternfly-4/react-core/src/components/Nav/Nav.js

+  /** Callback for when a list is expanded or collapsed */
+  onToggle: PropTypes.func
+};
+


Can we add a warning similar to other components for the a missing aria-label.

jschuler · 2018-09-28T22:09:04Z

@karelhala @tlabaj @amarie401 @dmiller9911 Refactored and made usage more explicit so user can pass in ids. Removed much of the conditional logic and cloning that was present before.

affects: @patternfly/react-core, @patternfly/react-docs ISSUES CLOSED: patternfly#547

jeff-phillips-18 · 2018-10-01T20:23:40Z

What is the migration path from patternfly-react vertical navigation component to this?

jschuler · 2018-10-01T21:08:03Z

I'm not sure @jeff-phillips-18, we'll need to compare and contrast and see where we fall short and where we want to make changes to the component to make the transition easier. Is there a workflow for this @LHinson ? Maybe a followup issue?

dmiller9911 · 2018-10-02T00:27:25Z

@jeff-phillips-18 I am definitely not trying to downplay migration for this, but aside from documentation is there much more needed? I assume that each application needing to migrate will only have 1 some navigation component along with the links. Migration for this component should be much lower effort than others.

jschuler requested review from amarie401, karelhala and tlabaj September 18, 2018 03:09

janwright73 added the review label Sep 18, 2018

amarie401 added the PF4 label Sep 18, 2018

karelhala reviewed Sep 19, 2018

View reviewed changes

tlabaj reviewed Sep 19, 2018

View reviewed changes

amarie401 reviewed Sep 19, 2018

View reviewed changes

tlabaj reviewed Sep 19, 2018

View reviewed changes

tlabaj requested changes Sep 19, 2018

View reviewed changes

amarie401 previously approved these changes Sep 25, 2018

View reviewed changes

jschuler dismissed amarie401’s stale review via 346f33c September 25, 2018 18:19

amarie401 previously approved these changes Sep 25, 2018

View reviewed changes

jschuler dismissed amarie401’s stale review via 509f6a0 September 25, 2018 18:59

amarie401 reviewed Sep 25, 2018

View reviewed changes

tlabaj reviewed Sep 25, 2018

View reviewed changes

karelhala reviewed Sep 26, 2018

View reviewed changes

tlabaj reviewed Sep 26, 2018

View reviewed changes

jschuler force-pushed the nav-system branch from b2e0d45 to e8acd30 Compare September 28, 2018 23:12

feat(pf4-nav): adds the nav component

31a5b50

affects: @patternfly/react-core, @patternfly/react-docs ISSUES CLOSED: patternfly#547

jschuler force-pushed the nav-system branch from 80a7743 to 31a5b50 Compare September 28, 2018 23:42

jschuler added 2 commits September 28, 2018 19:48

Merge remote-tracking branch 'upstream/master' into nav-system

e89bff9

add enzyme patch for context

9b17fb3

amarie401 approved these changes Oct 1, 2018

View reviewed changes

tlabaj approved these changes Oct 2, 2018

View reviewed changes

jschuler merged commit 677587c into patternfly:master Oct 2, 2018

janwright73 removed the review label Oct 2, 2018

priley86 added the enhancement 🚀 label Oct 2, 2018

wise-king-sullyman mentioned this pull request Jul 28, 2022

chore(Navigation): convert examples to TypeScript #7597

Merged

		@@ -0,0 +1 @@
		export { default as Avatar, AvatarProps } from './Avatar';

feat(pf4-nav): add nav component #626

feat(pf4-nav): add nav component #626

Uh oh!

Conversation

jschuler commented Sep 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

karelhala left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

karelhala commented Sep 19, 2018

Uh oh!

jschuler commented Sep 19, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jschuler commented Sep 19, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jschuler Sep 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amarie401 Sep 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jgiardino commented Sep 19, 2018

Uh oh!

jgiardino commented Sep 19, 2018

Uh oh!

jgiardino commented Sep 20, 2018

Uh oh!

mattnolting commented Sep 20, 2018

All <nav> elements

Expandable nav

As mentioned by @jgiardino

Uh oh!

jschuler commented Sep 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

element, it announces that it's clickable, but I can't find anything in the html that would indicate that it's clickable. Do you happen to know why JAWS thinks these headers are clickable?

Uh oh!

jschuler commented Sep 25, 2018

Uh oh!

mattnolting commented Sep 25, 2018

Uh oh!

coveralls commented Sep 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

jschuler commented Sep 18, 2018 •

edited

Loading

jschuler Sep 25, 2018 •

edited

Loading

amarie401 Sep 19, 2018 •

edited

Loading

All `<nav>` elements

jschuler commented Sep 25, 2018 •

edited

Loading

coveralls commented Sep 25, 2018 •

edited

Loading

tlabaj Sep 26, 2018 •

edited

Loading