Skip to content

Commit

Permalink
feat(nav): Updated search map title & icon (#1894)
Browse files Browse the repository at this point in the history
* feat(Nav): New search map icon + page title

* fix(icon-search-map): Remove fill, height + width

* fix(IconSearchMap): Restore height/width and fix underlying viewbox issue

the height and width on the raw SVG file were both set to 24 (matching the viewbox definition). Surprisngly, the image was being cut off. I found that removing the height/width properties resolved the issue. Looking further into it, the viewport was being removed by svgo - see [this](svg/svgo#1128) extensively discussed issue. Since we are re-scaling the svg with CSS to 20x20, we do need to preserve the image viewbox. Applying the solution from [this](svg/svgo#1128 (comment)) comment

* fix(GarageIcon): correct size

Going through all other icons to see which had a  viewbox that matched the width + height  and found only two: icon-garage and icon-up-right-arrow. icon-up-right-arrow had sizing explicitly set in CSS and was not affected. icon-garage defaulted to the leaflet default marker size of 12x12. This explicitly sets the iconSize to 21x25, matching what is specified in the SVG. I noticed that the width and height SVG properties are being removed as part of [svg-inline-loader](https://github.com/webpack-contrib/svg-inline-loader/tree/4bb5529fbdd847d7809067fe11840b48ee13dde4#removesvgtagattrs-boolean). I thought about turning off that behavior as part of this PR, but it seems like it could affect more icons and I'm not really sure if it is necessary

* test(SearchMapIcon): Add rendering test

* fix(SearchMapIcon): Adjust sizing/viewbox so not cut off
  • Loading branch information
KaylaBrady authored Jan 26, 2023
1 parent c663439 commit c5a4766
Show file tree
Hide file tree
Showing 16 changed files with 84 additions and 34 deletions.
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)

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")
})
})
2 changes: 2 additions & 0 deletions assets/tests/helpers/icon.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
RedLineIcon,
ReverseIcon,
SearchIcon,
SearchMapIcon,
TriangleDownIcon,
TriangleUpIcon,
UpDownIcon,
Expand Down Expand Up @@ -66,6 +67,7 @@ describe.each([
["RedLineIcon", RedLineIcon],
["ReverseIcon", ReverseIcon],
["SearchIcon", SearchIcon],
["SearchMapIcon", SearchMapIcon],
["TriangleDownIcon", TriangleDownIcon],
["TriangleUpIcon", TriangleUpIcon],
["UpDownIcon", UpDownIcon],
Expand Down
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,
})
}
}

0 comments on commit c5a4766

Please sign in to comment.