feat: Display empty detour table icon when the table is empty #2777

joshlarson · 2024-09-09T15:06:31Z

Before

Desktop

Mobile

After

Desktop

Mobile

Asana Ticket: https://app.asana.com/0/1205526445275136/1208226799631632/f

Depends on:

joshlarson · 2024-09-09T16:15:19Z

I originally did this work as two separate chunks - creating the icon (and the storybook story), and then adding it to the table, but then combined the work into this one PR.

If you'd prefer that I split this back into two PR's (see the two commits on this branch), lemme know - I'm happy to do it.

assets/src/components/detourListPage.tsx

github-actions · 2024-09-09T20:56:35Z

Coverage of commit `f922304`

Summary coverage rate:
  lines......: 93.0% (3299 of 3547 lines)
  functions..: 72.5% (1360 of 1876 functions)
  branches...: no data found

Files changed coverage rate: n/a

Download coverage report

assets/stories/skate-components/icons/emptyDetourTableIcon.stories.tsx

github-actions · 2024-09-10T14:16:02Z

Coverage of commit `2c933e0`

Summary coverage rate:
  lines......: 93.0% (3299 of 3547 lines)
  functions..: 72.5% (1360 of 1876 functions)
  branches...: no data found

Files changed coverage rate: n/a

Download coverage report

joshlarson · 2024-09-10T16:11:42Z

assets/css/detours/_detour_table.scss

@@ -61,6 +61,12 @@ $table-border-radius: 0.5rem;
    td:last-child {
      border-bottom-right-radius: $table-border-radius;
    }
+
+    @include media-breakpoint-down(md) {


This is an unrelated visual bug fix, but I noticed it while working on this PR and didn't realize until just now that I had accidentally lumped it into this PR.

I can split it out to a separate PR if you'd all prefer, but I didn't necessarily think it was worth the effort.

github-actions · 2024-09-10T18:57:00Z

Coverage of commit `26e6fb2`

Summary coverage rate:
  lines......: 93.0% (3318 of 3567 lines)
  functions..: 72.5% (1364 of 1882 functions)
  branches...: no data found

Files changed coverage rate: n/a

Download coverage report

hannahpurcell · 2024-09-11T14:59:04Z

assets/src/components/detoursTable.tsx

+)
+
+export const EmptyDetourTableIcon = (props: ComponentProps<"svg">) => (
+  <svg


I think I'd prefer this icon to be stored with the others at like static/images/icon-bus-circle.svg

There is previous conversion on why to prefer not doing it that way https://mbta.slack.com/archives/C047ZT8ACP5/p1725466195051749?thread_ts=1725464812.285259&cid=C047ZT8ACP5

Ohhh... I read this and then forgot about it once I saw the code out of context. Hmm, perhaps I should reply there? But how do we avoid distributing otherwise-reusable icons throughout the repo in a way that is hard to track?

Another worthwhile question: should this icon (and other icons) be moved somewhere more consistent?

I figured it was likely that this icon would only ever be used on this page, so it wasn't necessary to create a separate home for it, but I don't feel strongly about that.

maybe a skateIcons.tsx similar to bsIcons.tsx? Then we can have consistent tests for them. IMO This would be for "simple" icons which contain only SVG and don't take other props.

That makes sense to me. Or perhaps a folder that contains a Reacty icon component per file? I prefer surfing icons by file name, but that's maybe too-personal a preference.

Also, turning my feedback to non-blocking, since this had already been discussed (and I forgot), and the requested change is mainly an organizational one at this point

I like the bsIcons route because it makes it easy to apply uniform tests to all the icons without needing to edit the tests. But I'm open to other options.

assets/src/components/detoursTable.tsx

firestack · 2024-09-11T17:43:46Z

assets/src/components/detoursTable.tsx

+    <tbody>
+      <DetourRows data={data} status={status} />
+    </tbody>
+  </Table>
+)


nit: I'd prefer if we didn't abstract this yet, and avoid any prop drilling that we'd need to do by having this intermediate component.

Which part do you not want to abstract? I can't quite tell, but I think Github might be rendering this weird.

I was thinking that <DetourRows/> would be inlined here

github-actions · 2024-09-13T18:23:35Z

Coverage of commit `d7f3495`

Summary coverage rate:
  lines......: 93.0% (3318 of 3567 lines)
  functions..: 72.5% (1364 of 1882 functions)
  branches...: no data found

Files changed coverage rate: n/a

Download coverage report

assets/src/helpers/skateIcons.tsx

firestack

marking as changes requested for github overview screens

github-actions · 2024-09-23T20:34:34Z

Coverage of commit `5c17e94`

Summary coverage rate:
  lines......: 92.5% (3310 of 3580 lines)
  functions..: 71.4% (1366 of 1912 functions)
  branches...: no data found

Files changed coverage rate: n/a

Download coverage report

github-actions · 2024-09-23T21:04:02Z

Coverage of commit `5c17e94`

Summary coverage rate:
  lines......: 92.5% (3310 of 3580 lines)
  functions..: 71.4% (1366 of 1912 functions)
  branches...: no data found

Files changed coverage rate: n/a

Download coverage report

joshlarson force-pushed the jdl/feat/format-detour-list-table-on-mobile branch from 76c6374 to b4ba629 Compare September 9, 2024 15:10

joshlarson force-pushed the jdl/feat/empty-detour-table-icon branch from c7cfa7e to 8d4e0b6 Compare September 9, 2024 15:48

joshlarson force-pushed the jdl/feat/format-detour-list-table-on-mobile branch from 6ed52d0 to 7fca2cf Compare September 9, 2024 15:49

joshlarson force-pushed the jdl/feat/empty-detour-table-icon branch from 8d4e0b6 to 4b1a60e Compare September 9, 2024 15:49

joshlarson force-pushed the jdl/feat/format-detour-list-table-on-mobile branch from 7fca2cf to 8efa5ed Compare September 9, 2024 15:52

joshlarson force-pushed the jdl/feat/empty-detour-table-icon branch from 4b1a60e to ccff018 Compare September 9, 2024 15:52

joshlarson marked this pull request as ready for review September 9, 2024 15:55

joshlarson requested a review from a team as a code owner September 9, 2024 15:55

joshlarson marked this pull request as draft September 9, 2024 15:58

joshlarson force-pushed the jdl/feat/format-detour-list-table-on-mobile branch from 8efa5ed to b30b0d0 Compare September 9, 2024 16:01

joshlarson force-pushed the jdl/feat/empty-detour-table-icon branch from 4c1be6c to 67b3662 Compare September 9, 2024 16:01

joshlarson marked this pull request as ready for review September 9, 2024 16:02

Base automatically changed from jdl/feat/format-detour-list-table-on-mobile to main September 9, 2024 16:06

joshlarson force-pushed the jdl/feat/empty-detour-table-icon branch from 67b3662 to 38a1833 Compare September 9, 2024 16:13

joshlarson commented Sep 9, 2024

View reviewed changes

assets/src/components/detourListPage.tsx Outdated Show resolved Hide resolved

firestack reviewed Sep 10, 2024

View reviewed changes

assets/stories/skate-components/icons/emptyDetourTableIcon.stories.tsx Outdated Show resolved Hide resolved

joshlarson commented Sep 10, 2024

View reviewed changes

joshlarson force-pushed the jdl/feat/empty-detour-table-icon branch from 2c933e0 to 26e6fb2 Compare September 10, 2024 18:51

joshlarson requested a review from firestack September 10, 2024 18:58

joshlarson mentioned this pull request Sep 11, 2024

feat: Add properly-formatted headers to the detours list page #2785

Merged

hannahpurcell reviewed Sep 11, 2024

View reviewed changes

assets/src/components/detoursTable.tsx Outdated Show resolved Hide resolved

firestack added the deploy-to-dev-blue label Sep 11, 2024

firestack had a problem deploying to dev-blue September 11, 2024 17:31 — with GitHub Actions Failure

joshlarson temporarily deployed to dev-blue September 11, 2024 17:38 — with GitHub Actions Inactive

firestack reviewed Sep 11, 2024

View reviewed changes

joshlarson temporarily deployed to dev-blue September 13, 2024 18:02 — with GitHub Actions Inactive

joshlarson temporarily deployed to dev-blue September 13, 2024 18:18 — with GitHub Actions Inactive

joshlarson removed their assignment Sep 13, 2024

joshlarson requested a review from firestack September 13, 2024 19:29

firestack reviewed Sep 18, 2024

View reviewed changes

assets/src/helpers/skateIcons.tsx Outdated Show resolved Hide resolved

firestack requested changes Sep 19, 2024

View reviewed changes

firestack assigned joshlarson Sep 19, 2024

firestack force-pushed the jdl/feat/empty-detour-table-icon branch from d7f3495 to 2e70104 Compare September 19, 2024 16:06

joshlarson force-pushed the jdl/feat/empty-detour-table-icon branch 2 times, most recently from 4d29c4e to f808297 Compare September 23, 2024 16:02

joshlarson had a problem deploying to dev-blue September 23, 2024 16:02 — with GitHub Actions Failure

joshlarson force-pushed the jdl/feat/empty-detour-table-icon branch from f808297 to e015206 Compare September 23, 2024 16:09

joshlarson had a problem deploying to dev-blue September 23, 2024 16:09 — with GitHub Actions Failure

joshlarson force-pushed the jdl/feat/empty-detour-table-icon branch from 51cde35 to af3ad47 Compare September 23, 2024 18:44

joshlarson requested a review from firestack September 23, 2024 18:44

firestack approved these changes Sep 23, 2024

View reviewed changes

feat: Icon for empty detour list table

258ea06

joshlarson force-pushed the jdl/feat/empty-detour-table-icon branch from af3ad47 to a14a65f Compare September 23, 2024 19:15

joshlarson had a problem deploying to dev-blue September 23, 2024 19:15 — with GitHub Actions Failure

joshlarson enabled auto-merge (squash) September 23, 2024 20:10

joshlarson had a problem deploying to dev-blue September 23, 2024 20:11 — with GitHub Actions Failure

joshlarson temporarily deployed to dev-blue September 23, 2024 20:24 — with GitHub Actions Inactive

feat: Display empty detour table icon when the table is empty

5c17e94

joshlarson force-pushed the jdl/feat/empty-detour-table-icon branch from b2fe361 to 5c17e94 Compare September 23, 2024 20:29

joshlarson temporarily deployed to dev-blue September 23, 2024 20:35 — with GitHub Actions Inactive

joshlarson merged commit b1bcaf5 into main Sep 23, 2024
8 of 9 checks passed

joshlarson deleted the jdl/feat/empty-detour-table-icon branch September 23, 2024 20:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Display empty detour table icon when the table is empty #2777

feat: Display empty detour table icon when the table is empty #2777

joshlarson commented Sep 9, 2024 •

edited

Loading

joshlarson commented Sep 9, 2024

github-actions bot commented Sep 9, 2024

github-actions bot commented Sep 10, 2024

joshlarson Sep 10, 2024

github-actions bot commented Sep 10, 2024

hannahpurcell Sep 11, 2024

firestack Sep 11, 2024

hannahpurcell Sep 11, 2024

joshlarson Sep 11, 2024

firestack Sep 11, 2024

hannahpurcell Sep 11, 2024

firestack Sep 11, 2024

firestack Sep 11, 2024

joshlarson Sep 11, 2024

firestack Sep 11, 2024

joshlarson Sep 13, 2024

github-actions bot commented Sep 13, 2024

firestack left a comment

github-actions bot commented Sep 23, 2024

github-actions bot commented Sep 23, 2024

feat: Display empty detour table icon when the table is empty #2777

feat: Display empty detour table icon when the table is empty #2777

Conversation

joshlarson commented Sep 9, 2024 • edited Loading

Before

Desktop

Mobile

After

Desktop

Mobile

joshlarson commented Sep 9, 2024

github-actions bot commented Sep 9, 2024

Coverage of commit f922304

github-actions bot commented Sep 10, 2024

Coverage of commit 2c933e0

Choose a reason for hiding this comment

github-actions bot commented Sep 10, 2024

Coverage of commit 26e6fb2

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Sep 13, 2024

Coverage of commit d7f3495

firestack left a comment

Choose a reason for hiding this comment

github-actions bot commented Sep 23, 2024

Coverage of commit 5c17e94

github-actions bot commented Sep 23, 2024

Coverage of commit 5c17e94

joshlarson commented Sep 9, 2024 •

edited

Loading

Coverage of commit `f922304`

Coverage of commit `2c933e0`

Coverage of commit `26e6fb2`

Coverage of commit `d7f3495`

Coverage of commit `5c17e94`

Coverage of commit `5c17e94`