Skip to content
This repository has been archived by the owner on Mar 9, 2023. It is now read-only.

More edit controls #260

Merged
merged 5 commits into from
Nov 7, 2022
Merged

More edit controls #260

merged 5 commits into from
Nov 7, 2022

Conversation

dabreegster
Copy link
Contributor

#240

screencast.mp4
  1. A (+) button to insert a lane at a particular position
  2. Add missing card for sidewalks!
  3. Click direction arrow to toggle it

A next possible step is clicking the lane type icon to get a little modal popup and choose the type. So then inserting a lane and changing the type would become much more fluid

@@ -33,6 +35,13 @@
width: 50px;
}

.insert-lane {
position: absolute;
right: -15px;
Copy link
Contributor Author

@dabreegster dabreegster Nov 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot from 2022-11-07 12-47-12

Actually, I got the icons to be in a better place. Is this a reasonable way of achieving it?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect use of position: absolute! We want to anchor it to the bottom right of the card and that's idiomatic CSS to do so.

@github-actions
Copy link

github-actions bot commented Nov 7, 2022

Benchmark for 7126333

Click to view benchmark
Test Base PR %
tests/224637155 5.4±0.02µs 5.5±0.01µs +1.85%
tests/380103730 Japanese Expressway 7.1±0.01µs 7.1±0.01µs 0.00%
tests/389654080 9.0±0.01µs 9.0±0.02µs 0.00%
tests/49207928 cycleway:BACKWARD=lane 6.6±0.01µs 6.6±0.01µs 0.00%
tests/8591383 a bidirectional cycleway, oneway:bicycle 7.5±0.00µs 7.4±0.01µs -1.33%
tests/bus:lanes=designated| 6.8±0.02µs 6.8±0.02µs 0.00%
tests/busway=lane 6.2±0.02µs 6.2±0.02µs 0.00%
tests/cycleway=lane 6.3±0.01µs 6.4±0.07µs +1.59%
tests/cycleway=opposite oneway=yes oneway:bicycle=no 6.0±0.01µs 6.0±0.02µs 0.00%
tests/cycleway=opposite_track oneway=yes, deprecated 6.9±0.01µs 6.9±0.01µs 0.00%
tests/sidewalk:right=yes 5.6±0.01µs 5.7±0.02µs +1.79%
tests/sidewalk=both 5.8±0.02µs 5.8±0.01µs 0.00%

@github-actions
Copy link

github-actions bot commented Nov 7, 2022

Benchmark for d8065de

Click to view benchmark
Test Base PR %
tests/224637155 5.4±0.00µs 5.5±0.02µs +1.85%
tests/380103730 Japanese Expressway 7.1±0.01µs 7.2±0.08µs +1.41%
tests/389654080 9.0±0.01µs 9.0±0.01µs 0.00%
tests/49207928 cycleway:BACKWARD=lane 6.6±0.05µs 6.6±0.01µs 0.00%
tests/8591383 a bidirectional cycleway, oneway:bicycle 7.4±0.00µs 7.4±0.01µs 0.00%
tests/bus:lanes=designated| 6.8±0.01µs 6.8±0.02µs 0.00%
tests/busway=lane 6.1±0.02µs 6.2±0.04µs +1.64%
tests/cycleway=lane 6.3±0.01µs 6.3±0.04µs 0.00%
tests/cycleway=opposite oneway=yes oneway:bicycle=no 5.9±0.01µs 6.0±0.01µs +1.69%
tests/cycleway=opposite_track oneway=yes, deprecated 6.9±0.01µs 7.0±0.01µs +1.45%
tests/sidewalk:right=yes 5.5±0.02µs 5.6±0.01µs +1.82%
tests/sidewalk=both 5.7±0.01µs 5.8±0.02µs +1.75%

Copy link
Collaborator

@BudgieInWA BudgieInWA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Using a "dropdown" type interface for changing lane type as you describe sounds good.

@@ -11,7 +11,8 @@ export function makeLaneCard(lane, idx, app) {
}
node.appendChild(wrapInCenterDiv(width(lane)));

var finalRow = document.createElement("div");
var editRow = document.createElement("div");
editRow.align = "center";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer this done in CSS. text-align: center (for centering inline children of a block element) or margin: 0 auto (for centering a block element within its parent) will cover most cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

@dabreegster dabreegster merged commit 1f4d618 into main Nov 7, 2022
@dabreegster dabreegster deleted the more_edit_controls branch November 7, 2022 14:53
@github-actions
Copy link

github-actions bot commented Nov 7, 2022

Benchmark for 9cf476a

Click to view benchmark
Test Base PR %
tests/224637155 5.4±0.01µs 5.4±0.01µs 0.00%
tests/380103730 Japanese Expressway 7.1±0.02µs 7.2±0.08µs +1.41%
tests/389654080 8.8±0.01µs 8.8±0.01µs 0.00%
tests/49207928 cycleway:BACKWARD=lane 6.6±0.01µs 6.5±0.01µs -1.52%
tests/8591383 a bidirectional cycleway, oneway:bicycle 7.3±0.01µs 7.3±0.01µs 0.00%
tests/bus:lanes=designated| 6.7±0.01µs 6.7±0.01µs 0.00%
tests/busway=lane 6.0±0.01µs 6.0±0.01µs 0.00%
tests/cycleway=lane 6.2±0.01µs 6.2±0.01µs 0.00%
tests/cycleway=opposite oneway=yes oneway:bicycle=no 5.9±0.01µs 5.8±0.01µs -1.69%
tests/cycleway=opposite_track oneway=yes, deprecated 6.8±0.01µs 6.9±0.01µs +1.47%
tests/sidewalk:right=yes 5.4±0.01µs 5.4±0.01µs 0.00%
tests/sidewalk=both 5.7±0.01µs 5.7±0.01µs 0.00%

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants