Skip to content

Commit

Permalink
Use calc() in Sass (#201)
Browse files Browse the repository at this point in the history
One issue with our styling is that it's pretty haphazard. It's never
clear why certain values are chosen, and we often repeat values too.

By using `calc()`, we can make the code more self-documenting and
flexible. When you change a variable, it should correctly update all the
other styling code automatically.

--

This also recalibrates the icon sizes & removes the icon mixin, which
didn't end up working well with the project.

--

This PR should have no impact on the end user. It is only a refactor.
  • Loading branch information
Eric-Arellano authored Jul 3, 2024
1 parent 4a5f616 commit 0b4da10
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 52 deletions.
8 changes: 6 additions & 2 deletions index.html
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,14 @@
></select>
</div>
<div class="header-icons">
<div class="header-about-icon">
<div class="header-about-icon-container">
<i class="fa-solid fa-circle-info" title="About"></i>
</div>
<a href="https://parkingreform.org/parking-lot-map/" target="_blank">
<a
class="header-link-icon-container"
href="https://parkingreform.org/parking-lot-map/"
target="_blank"
>
<i
class="fa-solid fa-up-right-from-square"
title="View fullscreen"
Expand Down
33 changes: 22 additions & 11 deletions src/css/_controls.scss
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,32 @@
@use "theme/touchable-icons";
@use "theme/typography";

$label-font-size: typography.$font-size-md;
$zoom-controls-top-offset: 10px;
$outer-border-thickness: 2px;
$option-divider: 1px solid colors.$gray-light-translucent;

.leaflet-control-zoom.leaflet-bar,
.leaflet-control-layers.leaflet-control {
border: 2px solid colors.$gray-light-translucent;
border: $outer-border-thickness solid colors.$gray-light-translucent;
border-radius: 4px;
}

.leaflet-control-zoom.leaflet-bar {
top: 10px;
top: $zoom-controls-top-offset;

a {
width: touchable-icons.$min-touch-target;
height: touchable-icons.$min-touch-target;
font-size: typography.$font-size-md;
font-size: $label-font-size;
font-weight: normal;

display: flex;
align-items: center;
justify-content: center;

&:first-child {
border-bottom: 1px solid colors.$gray-light-translucent;
border-bottom: $option-divider;
}
}
}
Expand All @@ -33,8 +38,15 @@
}

#map > div.leaflet-control-container > div.leaflet-top.leaflet-right {
top: 104px;
margin-left: 10px;
$zoom-controls-height: calc(
(touchable-icons.$min-touch-target * 2) + $outer-border-thickness
);
$margin-between-zoom-control: 8px;
top: calc(
$zoom-controls-top-offset + $zoom-controls-height +
$margin-between-zoom-control
);
margin-left: 10px; // Mirrors zoom controls.
right: auto;
}

Expand All @@ -47,19 +59,18 @@
}

.leaflet-control-layers label {
font-size: typography.$font-size-md;
font-size: $label-font-size;
line-height: 1;

&:first-child {
border-bottom: 1px solid colors.$gray-light-translucent;
border-bottom: $option-divider;
}

display: flex;
align-items: center;

// Each option is naturally 18px, so 13px padding results in
// the touch target being the minimum size of 44px.
padding: 13px 8px;
$padding-y: calc((touchable-icons.$min-touch-target - $label-font-size) / 2);
padding: $padding-y 8px;

input[type="radio"] {
margin: 0;
Expand Down
2 changes: 1 addition & 1 deletion src/css/_header.scss
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ header {
color: colors.$white;
}

.header-about-icon {
.header-about-icon-container {
margin-right: 0.75em;
}

Expand Down
39 changes: 3 additions & 36 deletions src/css/theme/_touchable-icons.scss
Original file line number Diff line number Diff line change
@@ -1,39 +1,6 @@
$icon-size-xs: 20px;
$icon-size-sm: 24px;
$icon-size-md: 32px;
$icon-size-sm: 20px;
$icon-size-md: 24px;
$icon-size-lg: 32px;

// https://www.w3.org/WAI/WCAG21/Understanding/target-size.html
$min-touch-target: 44px;

@mixin touchable-icon($icon-size) {
width: $icon-size;
height: $icon-size;

// Ensure minimum touch target size
$touch-size: max($icon-size, $min-touch-target);
min-width: $touch-size;
min-height: $touch-size;

// Center the icon within the touch target
display: inline-flex;
align-items: center;
justify-content: center;

// Add padding if icon is smaller than min touch target
@if $icon-size < $min-touch-target {
$padding: ($min-touch-target - $icon-size) / 2;
padding: $padding;
}
}

@mixin touchable-icon-xs {
@include touchable-icon($icon-size-xs);
}

@mixin touchable-icon-sm {
@include touchable-icon($icon-size-sm);
}

@mixin touchable-icon-md {
@include touchable-icon($icon-size-md);
}
2 changes: 1 addition & 1 deletion src/js/about.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
const setUpAbout = () => {
const aboutElement: HTMLElement | null =
document.querySelector(".about-text-popup");
const infoButton = document.querySelector(".header-about-icon");
const infoButton = document.querySelector(".header-about-icon-container");
if (infoButton && aboutElement) {
infoButton.addEventListener("click", () => {
aboutElement.style.display =
Expand Down
2 changes: 1 addition & 1 deletion tests/app/about.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { expect, test } from "@playwright/test";
test("about popup can be opened and closed", async ({ page }) => {
await page.goto("");

const aboutIcon = ".header-about-icon";
const aboutIcon = ".header-about-icon-container";

const aboutIsVisible = async () =>
page.$eval(".about-text-popup", (el) => el.style.display === "block");
Expand Down

0 comments on commit 0b4da10

Please sign in to comment.