Skip to content

Commit

Permalink
fix: Render UI navigation items as links instead of buttons (#4970)
Browse files Browse the repository at this point in the history
fix: Render navigation items as links instead of buttons

Using link is semantically correct and improves accessibility, allows
opening the linked pages in another tab etc.

Use react-router-dom Links instead of plain anchors to prevent full page
reloads when navigating.

Signed-off-by: Harri Lehtola <[email protected]>
  • Loading branch information
peruukki authored Jan 28, 2025
1 parent 8ac6a85 commit 1267703
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 24 deletions.
4 changes: 2 additions & 2 deletions ui/src/FeastUISansProviders.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ test("routes are reachable", async () => {

const routeRegExp = new RegExp(routeName, "i");

await user.click(screen.getByRole("button", { name: routeRegExp }));
await user.click(screen.getByRole("link", { name: routeRegExp }));

// Should land on a page with the heading
screen.getByRole("heading", {
Expand All @@ -112,7 +112,7 @@ test("features are reachable", async () => {
await screen.findByText(/Explore this Project/i);
const routeRegExp = new RegExp("Feature Views", "i");

await user.click(screen.getByRole("button", { name: routeRegExp }));
await user.click(screen.getByRole("link", { name: routeRegExp }));

screen.getByRole("heading", {
name: "Feature Views",
Expand Down
30 changes: 8 additions & 22 deletions ui/src/pages/Sidebar.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React, { useContext, useState } from "react";

import { EuiIcon, EuiSideNav, htmlIdGenerator } from "@elastic/eui";
import { useNavigate, useParams } from "react-router-dom";
import { Link, useParams } from "react-router-dom";
import { useMatchSubpath } from "../hooks/useMatchSubpath";
import useLoadRegistry from "../queries/useLoadRegistry";
import RegistryPathContext from "../contexts/RegistryPathContext";
Expand All @@ -19,8 +19,6 @@ const SideNav = () => {

const [isSideNavOpenOnMobile, setisSideNavOpenOnMobile] = useState(false);

const navigate = useNavigate();

const toggleOpenOnMobile = () => {
setisSideNavOpenOnMobile(!isSideNavOpenOnMobile);
};
Expand Down Expand Up @@ -57,57 +55,45 @@ const SideNav = () => {

const baseUrl = `${process.env.PUBLIC_URL || ""}/p/${projectName}`;

const sideNav = [
const sideNav: React.ComponentProps<typeof EuiSideNav>['items'] = [
{
name: "Home",
id: htmlIdGenerator("basicExample")(),
onClick: () => {
navigate(`${baseUrl}/`);
},
renderItem: props => <Link {...props} to={`${baseUrl}/`} />,
items: [
{
name: dataSourcesLabel,
id: htmlIdGenerator("dataSources")(),
icon: <EuiIcon type={DataSourceIcon} />,
onClick: () => {
navigate(`${baseUrl}/data-source`);
},
renderItem: props => <Link {...props} to={`${baseUrl}/data-source`} />,
isSelected: useMatchSubpath(`${baseUrl}/data-source`),
},
{
name: entitiesLabel,
id: htmlIdGenerator("entities")(),
icon: <EuiIcon type={EntityIcon} />,
onClick: () => {
navigate(`${baseUrl}/entity`);
},
renderItem: props => <Link {...props} to={`${baseUrl}/entity`} />,
isSelected: useMatchSubpath(`${baseUrl}/entity`),
},
{
name: featureViewsLabel,
id: htmlIdGenerator("featureView")(),
icon: <EuiIcon type={FeatureViewIcon} />,
onClick: () => {
navigate(`${baseUrl}/feature-view`);
},
renderItem: props => <Link {...props} to={`${baseUrl}/feature-view`} />,
isSelected: useMatchSubpath(`${baseUrl}/feature-view`),
},
{
name: featureServicesLabel,
id: htmlIdGenerator("featureService")(),
icon: <EuiIcon type={FeatureServiceIcon} />,
onClick: () => {
navigate(`${baseUrl}/feature-service`);
},
renderItem: props => <Link {...props} to={`${baseUrl}/feature-service`} />,
isSelected: useMatchSubpath(`${baseUrl}/feature-service`),
},
{
name: savedDatasetsLabel,
id: htmlIdGenerator("savedDatasets")(),
icon: <EuiIcon type={DatasetIcon} />,
onClick: () => {
navigate(`${baseUrl}/data-set`);
},
renderItem: props => <Link {...props} to={`${baseUrl}/data-set`} />,
isSelected: useMatchSubpath(`${baseUrl}/data-set`),
},
],
Expand Down

0 comments on commit 1267703

Please sign in to comment.