-
Notifications
You must be signed in to change notification settings - Fork 7
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: Move and style copyButton, add to active and past detour views #2852
base: main
Are you sure you want to change the base?
Changes from all commits
1f0122e
9a97b3f
0d6241b
5b2ac4a
27b42f9
31bdf28
0702632
e7431c2
4e20a0c
30cd45e
63bbf40
986752b
71b81e6
b0d3ffa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,10 +44,14 @@ | |
.l-diversion-page__header { | ||
border-bottom: none; | ||
} | ||
|
||
.l-diversion-page-panel__header { | ||
align-items: center; | ||
} | ||
|
||
.c-diversion-panel__header_text, | ||
.c-detour-panel__subheader, | ||
.c-diversion-panel__back-button-container, | ||
.c-active-detour__alert-icon { | ||
.c-diversion-panel__desktop-buttons { | ||
display: none !important; | ||
} | ||
} | ||
|
@@ -89,7 +93,7 @@ | |
} | ||
} | ||
|
||
.l-diversion-page-panel__header .c-diversion-panel__back-button { | ||
.l-diversion-page-panel__header .c-diversion-panel__outline-button--back { | ||
display: none; | ||
} | ||
} | ||
|
@@ -161,3 +165,14 @@ | |
height: 1.5rem; | ||
width: 1.5rem; | ||
} | ||
|
||
.c-diversion-panel__outline-button { | ||
height: 2rem; | ||
border-radius: 0.2rem; | ||
font-size: 0.875rem; | ||
padding: 0.375rem 0.5rem; | ||
|
||
&--copy { | ||
margin: 0.78125rem 0; | ||
} | ||
Comment on lines
+175
to
+177
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this margin should be handled via the |
||
} | ||
Comment on lines
+169
to
+178
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we need this? this seems like a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh my gosh, I totally reinvented the wheel. Thanks!! |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,8 +1,15 @@ | ||||||
import React from "react" | ||||||
import { Stop } from "../../schedule" | ||||||
import { Badge, ListGroup } from "react-bootstrap" | ||||||
import { | ||||||
Badge, | ||||||
Button, | ||||||
ListGroup, | ||||||
OverlayTrigger, | ||||||
Popover, | ||||||
} from "react-bootstrap" | ||||||
import { uniqBy } from "../../helpers/array" | ||||||
import { RoutePill } from "../routePill" | ||||||
import { Files } from "../../helpers/bsIcons" | ||||||
|
||||||
interface MissedStopsProps { | ||||||
missedStops?: Stop[] | ||||||
|
@@ -64,3 +71,26 @@ export const AffectedRoute = ({ | |||||
</div> | ||||||
</section> | ||||||
) | ||||||
|
||||||
export const CopyButton = ({ detourText }: { detourText: string }) => ( | ||||||
<OverlayTrigger | ||||||
placement="bottom" | ||||||
trigger="click" | ||||||
rootClose | ||||||
rootCloseEvent="mousedown" | ||||||
overlay={ | ||||||
<Popover> | ||||||
<Popover.Body>Copied to clipboard!</Popover.Body> | ||||||
</Popover> | ||||||
} | ||||||
> | ||||||
<Button | ||||||
className="c-diversion-panel__outline-button c-diversion-panel__outline-button--copy icon-link" | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Between these two suggestions
I don't think most of the classes are needed?
Suggested change
|
||||||
variant="outline-primary" | ||||||
onClick={() => window.navigator.clipboard?.writeText(detourText)} | ||||||
> | ||||||
<Files /> | ||||||
Copy details | ||||||
</Button> | ||||||
</OverlayTrigger> | ||||||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: why change the name of this button? IMO the "outline" info doesn't seem necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're totally right. I renamed at one point, assuming I'd use the
--back
as an extension of outline button, as in these lines, but not needed. (Plus, I see those might need tweaking as well)