Skip to content

Commit

Permalink
Improve one-shot map importing UI (partly motivated by #760):
Browse files Browse the repository at this point in the history 
- stop overwriting the one zz/overpass map by naming them differently
- allow a user-specified name too
- move the buttons to search all maps and import a new place to the top
  of the ever-growing country list
  • Loading branch information
@dabreegster
dabreegster committed Sep 23, 2021
1 parent f17f4c2 commit 87dd029
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 61 deletions.
9 changes: 7 additions & 2 deletions cli/src/main.rs
View file
Expand Up @@ -146,8 +146,12 @@ enum Command {
/// Imports a one-shot A/B Street map in a single command.
OneStepImport {
/// The path to a GeoJSON file with a boundary
#[structopt()]
#[structopt(long)]
geojson_path: String,
/// What to name the new imported map. The country will always be "zz" (a fake country
/// code), with the city as "oneshot." This name shouldn't contain spaces or be empty.
#[structopt(long)]
map_name: String,
/// Do people drive on the left side of the road in this map?
#[structopt(long)]
drive_on_left: bool,
Expand Down Expand Up @@ -206,9 +210,10 @@ async fn main() -> Result<()> {
}
Command::OneStepImport {
geojson_path,
map_name,
drive_on_left,
use_geofabrik,
} => one_step_import::run(geojson_path, drive_on_left, use_geofabrik).await?,
} => one_step_import::run(geojson_path, map_name, drive_on_left, use_geofabrik).await?,
Command::Import { raw_args } => importer::run(raw_args).await,
}
Ok(())
Expand Down
26 changes: 14 additions & 12 deletions cli/src/one_step_import.rs
View file
Expand Up @@ -5,7 +5,19 @@ use anyhow::Result;
use abstio::CityName;
use geom::LonLat;

pub async fn run(geojson_path: String, drive_on_left: bool, use_geofabrik: bool) -> Result<()> {
pub async fn run(
geojson_path: String,
name: String,
drive_on_left: bool,
use_geofabrik: bool,
) -> Result<()> {
if name.contains(" ") || name.is_empty() {
panic!(
"--map_name must be non-empty and contain no spaces: {}",
name
);
}

// Convert to a boundary polygon.
{
println!("Converting GeoJSON to Osmosis boundary");
Expand All @@ -22,11 +34,9 @@ pub async fn run(geojson_path: String, drive_on_left: bool, use_geofabrik: bool)
}

let city = CityName::new("zz", "oneshot");
let name;
let osm;
if !use_geofabrik {
// No easy guess on this without looking at the XML file
name = "overpass".to_string();
println!("Downloading OSM data from Overpass...");
osm = city.input_path(format!("osm/{}.osm", name));

let geojson = abstio::slurp_file(geojson_path)?;
Expand All @@ -48,11 +58,6 @@ pub async fn run(geojson_path: String, drive_on_left: bool, use_geofabrik: bool)
println!("Figuring out what Geofabrik file contains your boundary");
let url = crate::pick_geofabrik::run("boundary0.poly".to_string()).await?;

// Name the temporary map based on the Geofabrik region.
name = abstutil::basename(&url)
.strip_suffix("-latest.osm")
.unwrap()
.to_string();
let pbf = city.input_path(format!("osm/{}.pbf", abstutil::basename(&url)));
osm = city.input_path(format!("osm/{}.osm", name));
std::fs::create_dir_all(std::path::Path::new(&osm).parent().unwrap())
Expand Down Expand Up @@ -87,8 +92,5 @@ pub async fn run(geojson_path: String, drive_on_left: bool, use_geofabrik: bool)
// debugging.
abstio::delete_file("boundary0.poly");

// For the sake of the UI, print the name of the new map as the last line of output.
println!("{}", name);

Ok(())
}
29 changes: 13 additions & 16 deletions map_gui/src/tools/city_picker.rs
View file
Expand Up @@ -4,9 +4,9 @@ use abstio::{CityName, Manifest, MapName};
use geom::{Distance, Percent};
use map_model::City;
use widgetry::{
Autocomplete, ClickOutcome, ControlState, DrawBaselayer, DrawWithTooltips, EventCtx, GeomBatch,
GfxCtx, Image, Key, Line, Outcome, Panel, RewriteColor, State, Text, TextExt, Transition,
Widget,
lctrl, Autocomplete, ClickOutcome, ControlState, DrawBaselayer, DrawWithTooltips, EventCtx,
GeomBatch, GfxCtx, Image, Key, Line, Outcome, Panel, RewriteColor, State, Text, TextExt,
Transition, Widget,
};

use crate::load::{FileLoader, MapLoader};
Expand Down Expand Up @@ -135,13 +135,6 @@ impl<A: AppLike + 'static> CityPicker<A> {
);
}
}
other_places.push(
ctx.style()
.btn_outline
.text("Search all maps")
.hotkey(Key::Tab)
.build_def(ctx),
);

Transition::Replace(Box::new(CityPicker {
on_load: Some(on_load),
Expand All @@ -150,12 +143,6 @@ impl<A: AppLike + 'static> CityPicker<A> {
Line("Select a district").small_heading().into_widget(ctx),
ctx.style().btn_close_widget(ctx),
]),
Widget::row(vec![
Widget::col(other_places).centered_vert(),
district_picker,
Widget::col(this_city).centered_vert(),
]),
"Don't see the place you want?".text_widget(ctx),
if cfg!(target_arch = "wasm32") {
// On web, this is a link, so it's styled appropriately.
ctx.style()
Expand All @@ -171,6 +158,16 @@ impl<A: AppLike + 'static> CityPicker<A> {
.text("Import a new city into A/B Street")
.build_widget(ctx, "import new city")
},
ctx.style()
.btn_outline
.icon_text("system/assets/tools/search.svg", "Search all maps")
.hotkey(lctrl(Key::F))
.build_def(ctx),
Widget::row(vec![
Widget::col(other_places).centered_vert(),
district_picker,
Widget::col(this_city).centered_vert(),
]),
]))
.build(ctx),
}))
Expand Down
72 changes: 41 additions & 31 deletions map_gui/src/tools/importer.rs
View file
Expand Up @@ -5,7 +5,7 @@ use clipboard::{ClipboardContext, ClipboardProvider};

use abstio::MapName;
use widgetry::{
EventCtx, GfxCtx, Line, Outcome, Panel, State, TextExt, Toggle, Transition, Widget,
EventCtx, GfxCtx, Line, Outcome, Panel, State, TextBox, TextExt, Toggle, Transition, Widget,
};

use crate::load::MapLoader;
Expand Down Expand Up @@ -49,29 +49,24 @@ impl<A: AppLike + 'static> ImportCity<A> {
"Copy the JSON text on the right into your clipboard".text_widget(ctx),
])
.margin_below(16),
Toggle::choice(
ctx,
"left handed driving",
"drive on the left",
"right",
None,
false,
),
Toggle::choice(ctx, "source", "GeoFabrik", "Overpass (faster)", None, false),
Widget::row(vec![
"Step 4)".text_widget(ctx).centered_vert(),
Toggle::choice(
ctx,
"left handed driving",
"drive on the left",
"right",
None,
false,
),
"Name the map:".text_widget(ctx).centered_vert(),
TextBox::widget(ctx, "new_map_name", generate_new_map_name(), true, 20),
]),
Widget::row(vec![
"Step 5)".text_widget(ctx).centered_vert(),
Toggle::choice(ctx, "source", "GeoFabrik", "Overpass (faster)", None, false),
]),
Widget::row(vec![
"Step 6)".text_widget(ctx).centered_vert(),
ctx.style()
.btn_solid_primary
.text("Import the area from your clipboard")
.build_def(ctx),
])
.margin_below(32),
ctx.style()
.btn_solid_primary
.text("Import the area from your clipboard")
.build_def(ctx)
.margin_below(32),
ctx.style()
.btn_plain
.btn()
Expand Down Expand Up @@ -102,35 +97,35 @@ impl<A: AppLike + 'static> State<A> for ImportCity<A> {
Transition::Keep
}
"Import the area from your clipboard" => {
let name = sanitize_name(self.panel.text_box("new_map_name"));

let mut args = vec![
find_exe("cli"),
"one-step-import".to_string(),
"boundary.geojson".to_string(),
"--geojson-path=boundary.geojson".to_string(),
format!("--map-name={}", name),
];
if self.panel.is_checked("left handed driving") {
args.push("--drive_on_left".to_string());
args.push("--drive-on-left".to_string());
}
if self.panel.is_checked("source") {
args.push("--use_geofabrik".to_string());
args.push("--use-geofabrik".to_string());
}
match grab_geojson_from_clipboard() {
Ok(()) => Transition::Push(crate::tools::RunCommand::new_state(
ctx,
true,
args,
Box::new(|_, _, success, mut lines| {
Box::new(|_, _, success, _| {
if success {
abstio::delete_file("boundary.geojson");

Transition::ConsumeState(Box::new(move |state, ctx, app| {
let mut state =
state.downcast::<ImportCity<A>>().ok().unwrap();
let on_load = state.on_load.take().unwrap();
// one_step_import prints the name of the map as the last
// line.
let name =
MapName::new("zz", "oneshot", &lines.pop().unwrap());
vec![MapLoader::new_state(ctx, app, name, on_load)]
let map_name = MapName::new("zz", "oneshot", &name);
vec![MapLoader::new_state(ctx, app, map_name, on_load)]
}))
} else {
// The popup already explained the failure
Expand Down Expand Up @@ -179,3 +174,18 @@ fn grab_geojson_from_clipboard() -> Result<()> {
write!(f, "{}", contents)?;
Ok(())
}

fn sanitize_name(x: String) -> String {
x.replace(" ", "_")
}

fn generate_new_map_name() -> String {
let mut i = 0;
loop {
let name = format!("imported_{}", i);
if !abstio::file_exists(MapName::new("zz", "oneshot", &name).path()) {
return name;
}
i += 1;
}
}

0 comments on commit 87dd029

Please sign in to comment.