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

feat(nav): Updated search map title & icon #1894

Merged
merged 6 commits into from
Jan 26, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions assets/src/components/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,12 @@ export const AppRoutes = () => {
element={<ShuttleMapPage />}
/>
<BrowserRoute path="/settings" element={<SettingsPage />} />
{mapMode.title === "Search" ? (
{mapMode.supportsRightPanel ? (
<BrowserRoute path={mapMode.path} element={mapMode.element} />
) : null}
</Route>
<Route>
{mapMode.title === "Map" ? (
{!mapMode.supportsRightPanel ? (
<BrowserRoute path={mapMode.path} element={mapMode.element} />
) : null}
</Route>
Expand Down
1 change: 1 addition & 0 deletions assets/src/components/mapMarkers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,7 @@ const garageLeafletIcon = Leaflet.divIcon({
html: garageIcon,
className: "m-garage-icon",
iconAnchor: new Leaflet.Point(10, 25),
iconSize: [21, 25],
})

const Garage = ({
Expand Down
4 changes: 2 additions & 2 deletions assets/src/components/nav/bottomNavMobile.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from "react"
import { LadderIcon, MapIcon, SearchIcon, SwingIcon } from "../../helpers/icon"
import { LadderIcon, MapIcon, SwingIcon } from "../../helpers/icon"
import { NavLink } from "react-router-dom"
import { tagManagerEvent } from "../../helpers/googleTagManager"
import { mapModeForUser } from "../../util/mapMode"
Expand Down Expand Up @@ -57,7 +57,7 @@ const BottomNavMobile: React.FC<Props> = ({
title={mapMode.title}
to={mapMode.path}
>
<SearchIcon className="m-bottom-nav-mobile__icon" />
<mapMode.navIcon className="m-bottom-nav-mobile__icon" />
</NavLink>
</li>

Expand Down
3 changes: 1 addition & 2 deletions assets/src/components/nav/leftNav.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import NotificationBellIcon from "../notificationBellIcon"
import {
LadderIcon,
MapIcon,
SearchIcon,
LateIcon,
SwingIcon,
DoubleChevronRightIcon,
Expand Down Expand Up @@ -111,7 +110,7 @@ const LeftNav = ({
title={mapMode.title}
to={mapMode.path}
>
<SearchIcon className="m-left-nav__icon" />
<mapMode.navIcon className="m-left-nav__icon" />
{collapsed ? null : mapMode.title}
</NavLink>
</li>
Expand Down
10 changes: 4 additions & 6 deletions assets/src/components/nav/topNavMobile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import NotificationBellIcon from "../notificationBellIcon"
import { currentTabName, RouteTab } from "../../models/routeTab"
import NavMenu from "./navMenu"
import { tagManagerEvent } from "../../helpers/googleTagManager"
import { searchMapConfig } from "../../util/mapMode"

export const toTitleCase = (str: string): string => {
return str.replace(
Expand All @@ -18,12 +19,9 @@ export const pageOrTabName = (
pathname: string,
routeTabs: RouteTab[]
): string => {
let tabName = "Skate"

if (pathname === "/") tabName = currentTabName(routeTabs)
else tabName = toTitleCase(pathname.replace("/", "").replace("-", " "))

return tabName
if (pathname === "/") return currentTabName(routeTabs)
if (pathname === searchMapConfig.path) return searchMapConfig.title
else return toTitleCase(pathname.replace("/", "").replace("-", " "))
}

interface Props {
Expand Down
4 changes: 4 additions & 0 deletions assets/src/helpers/icon.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ import saveIconSvg from "../../static/images/icon-save.svg"
// @ts-ignore
import searchIconSvg from "../../static/images/icon-search.svg"
// @ts-ignore
import searchMapIconSvg from "../../static/images/icon-search-map.svg"
// @ts-ignore
import settingsIconSvg from "../../static/images/icon-settings.svg"
// @ts-ignore
import speechBubbleIconSvg from "../../static/images/icon-speech-bubble.svg"
Expand Down Expand Up @@ -215,6 +217,8 @@ export const SaveIcon = svgIcon(saveIconSvg)

export const SearchIcon = svgIcon(searchIconSvg)

export const SearchMapIcon = svgIcon(searchMapIconSvg)
Copy link
Member

Choose a reason for hiding this comment

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

suggestion: it's probably worth adding this icon to the list in icon.test.tsx, even if I have my doubts about how useful those particular tests are.


export const SettingsIcon = svgIcon(settingsIconSvg)

export const SpeechBubbleIcon = svgIcon(speechBubbleIconSvg)
Expand Down
23 changes: 20 additions & 3 deletions assets/src/util/mapMode.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,32 @@ import React from "react"
import { ReactElement } from "react"
import MapPage from "../components/mapPage"
import SearchPage from "../components/searchPage"
import { SearchIcon, SearchMapIcon } from "../helpers/icon"
import inTestGroup, { MAP_BETA_GROUP_NAME } from "../userInTestGroup"

export interface NavMode {
path: string
title: string
element: ReactElement
navIcon: (props: any) => ReactElement
supportsRightPanel: boolean
}

const legacyMapConfig = {
path: "/search",
title: "Search",
element: <SearchPage />,
navIcon: SearchIcon,
supportsRightPanel: true,
}

export const searchMapConfig = {
path: "/map",
title: "Search Map",
element: <MapPage />,
navIcon: SearchMapIcon,
supportsRightPanel: false,
}

export const mapModeForUser = (): NavMode =>
inTestGroup(MAP_BETA_GROUP_NAME)
? { path: "/map", title: "Map", element: <MapPage /> }
: { path: "/search", title: "Search", element: <SearchPage /> }
inTestGroup(MAP_BETA_GROUP_NAME) ? searchMapConfig : legacyMapConfig
4 changes: 4 additions & 0 deletions assets/static/images/icon-search-map.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
5 changes: 0 additions & 5 deletions assets/svgo.yml

This file was deleted.

4 changes: 2 additions & 2 deletions assets/tests/components/nav.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ describe("Nav", () => {
expect(result.queryByText("Route Ladders")).toBeNull()
})

test("renders nav item with title 'Map' if in map test group", () => {
test("renders nav item with title 'Search Map' if in map test group", () => {
;(getTestGroups as jest.Mock).mockReturnValue([MAP_BETA_GROUP_NAME])

render(
Expand All @@ -83,7 +83,7 @@ describe("Nav", () => {
)

expect(screen.queryByTitle("Search")).toBeNull()
expect(screen.queryByTitle("Map")).toBeInTheDocument()
expect(screen.queryByTitle("Search Map")).toBeInTheDocument()
})

test("renders desktop nav content", () => {
Expand Down
4 changes: 2 additions & 2 deletions assets/tests/components/nav/bottomNavMobile.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ describe("BottomNavMobile", () => {
expect(tagManagerEvent).toHaveBeenCalledWith("swings_view_toggled")
})

test("renders nav item with title 'Map' if in map test group", () => {
test("renders nav item with title 'Search Map' if in map test group", () => {
;(getTestGroups as jest.Mock).mockReturnValue([MAP_BETA_GROUP_NAME])

render(
Expand All @@ -50,6 +50,6 @@ describe("BottomNavMobile", () => {
)

expect(screen.queryByTitle("Search")).toBeNull()
expect(screen.queryByTitle("Map")).toBeInTheDocument()
expect(screen.queryByTitle("Search Map")).toBeInTheDocument()
})
})
4 changes: 2 additions & 2 deletions assets/tests/components/nav/leftNav.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ describe("LeftNav", () => {
expect(result.queryByTitle("Expand")).not.toBeNull()
})

test("renders nav item with title 'Map' if in map test group", () => {
test("renders nav item with title 'Search Map' if in map test group", () => {
;(getTestGroups as jest.Mock).mockReturnValueOnce([MAP_BETA_GROUP_NAME])

render(
Expand All @@ -94,7 +94,7 @@ describe("LeftNav", () => {
)

expect(screen.queryByTitle("Search")).toBeNull()
expect(screen.getByTitle("Map")).toBeInTheDocument()
expect(screen.getByTitle("Search Map")).toBeInTheDocument()
})

test("can toggle nav menu on tablet layout", async () => {
Expand Down
4 changes: 4 additions & 0 deletions assets/tests/components/nav/topNavMobile.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,4 +75,8 @@ describe("pageOrTabName", () => {
test("returns page name for shuttle map", () => {
expect(pageOrTabName("/shuttle-map", [])).toEqual("Shuttle Map")
})

test("returns the custom title for the search map page", () => {
expect(pageOrTabName("/map", [])).toEqual("Search Map")
})
})
7 changes: 6 additions & 1 deletion assets/tests/util/mapMode.test.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from "react"
import MapPage from "../../src/components/mapPage"
import SearchPage from "../../src/components/searchPage"
import { SearchIcon, SearchMapIcon } from "../../src/helpers/icon"
import { MAP_BETA_GROUP_NAME } from "../../src/userInTestGroup"
import getTestGroups from "../../src/userTestGroups"
import { mapModeForUser } from "../../src/util/mapMode"
Expand All @@ -15,8 +16,10 @@ describe("mapModeForUser", () => {
;(getTestGroups as jest.Mock).mockReturnValueOnce([MAP_BETA_GROUP_NAME])
expect(mapModeForUser()).toEqual({
path: "/map",
title: "Map",
title: "Search Map",
element: <MapPage />,
navIcon: SearchMapIcon,
supportsRightPanel: false,
})
})

Expand All @@ -27,6 +30,8 @@ describe("mapModeForUser", () => {
path: "/search",
title: "Search",
element: <SearchPage />,
navIcon: SearchIcon,
supportsRightPanel: true,
})
})
})
35 changes: 28 additions & 7 deletions assets/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,26 @@ const path = require("path")
const glob = require("glob")
const MiniCssExtractPlugin = require("mini-css-extract-plugin")
const TerserPlugin = require("terser-webpack-plugin")
const CssMinimizerPlugin = require("css-minimizer-webpack-plugin");
const CssMinimizerPlugin = require("css-minimizer-webpack-plugin")
const CopyWebpackPlugin = require("copy-webpack-plugin")

module.exports = (env, options) => {
const plugins = [
new MiniCssExtractPlugin({ filename: "../css/app.css" }),
new CopyWebpackPlugin({ patterns: [{ from: "static/", to: "../" }] })
new CopyWebpackPlugin({ patterns: [{ from: "static/", to: "../" }] }),
]

const useMinimization = options.mode === "production";
const useMinimization = options.mode === "production"

return({
return {
optimization: {
minimize: useMinimization,
minimizer: [
new TerserPlugin({ parallel: true }),
new CssMinimizerPlugin(),
],
},
devtool: 'source-map',
devtool: "source-map",
entry: {
"./js/app.tsx": ["./src/app.tsx"].concat(glob.sync("./vendor/**/*.js")),
},
Expand Down Expand Up @@ -59,7 +59,28 @@ module.exports = (env, options) => {
{
loader: "svgo-loader",
options: {
externalConfig: "svgo.yml",
plugins: [
{
name: "preset-default",
params: {
overrides: {
// viewBox is required to resize SVGs with CSS.
// @see https://github.com/svg/svgo/issues/1128
removeViewBox: false,
},
},
},
{
name: "removeTitle",
active: true,
},
{
name: "removeAttrs",
params: {
attrs: ["id"],
},
},
],
},
},
],
Expand All @@ -75,5 +96,5 @@ module.exports = (env, options) => {
modules: ["deps", "node_modules"],
},
plugins: plugins,
})
}
}