feat: Add Activate button gated behind detours-list test group
#2713
Conversation
Coverage of commit
|
joshlarson
changed the title
Activate button gated behind detours-list test group
assets/css/_diversion_page.scss
Outdated
|
|
||
| .c-diversion-panel__activate-button { | ||
| background-color: semantic.$missed-stop; | ||
| } |
There was a problem hiding this comment.
Do we need bespoke css for this? I'd prefer to use a variant on the button itself https://react-bootstrap.github.io/docs/components/buttons
There was a problem hiding this comment.
Ahh - I didn't realize I could do that
(It gives it nicer hover behavior too!)
| @@ -0,0 +1,86 @@ | |||
| import React from "react" | |||
There was a problem hiding this comment.
This is a thing I've been thinking about doing for a while.
diversionPage.test.tsx is getting very large, which makes it kind of unwieldy. There's no reason we need to have all of the detour tests live in the same file. I was thinking that we could start splitting out test cases for different angles of the detours feature into different test files.
For instance, all of the test cases that make assertions about which route-shape pieces are visible could be in diversionPageRouteShapes.test.tsx, and the test cases that assert when the undo and clear buttons are disabled or enabled might live in diversionPageUndoClearButtons.test.tsx.
Exactly how to divvy the use cases into files is a matter of taste, and I don't have strong feelings about it. But I figured that test cases related to saving/activating might be happiest away from everything else.
Coverage of commit
|
Coverage of commit
|
| @@ -233,6 +233,11 @@ export const DiversionPage = ({ | |||
| onNavigateBack={editDetour} | |||
| detourText={textArea} | |||
| onChangeDetourText={setTextArea} | |||
| onActivateDetour={ | |||
| inTestGroup(TestGroups.DetoursList) | |||
| ? () => alert("Activate!") | |||
There was a problem hiding this comment.
nit: I can't quite articulate why, so I guess take this as non-blocking, but I'd prefer to not use alert if we don't really need it.
There was a problem hiding this comment.
Does knowing that this is only a temporary thing change your feeling? The alert is only in lieu of having hi-fi designs for this action
There was a problem hiding this comment.
Slightly, I'm fine with it being non-blocking, but I'm inclined to enable the https://eslint.org/docs/latest/rules/no-alert rule
There was a problem hiding this comment.
Oh I'm happy to remove it - I used alert because it was an easy way to locally test it, but I'm not attached.
Coverage of commit
|
Asana Ticket: https://app.asana.com/0/1205526445275136/1207935854187629/f
Subtask Of: https://app.asana.com/0/1148853526253420/1205836718156544/f