Skip to content
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

Reorganize export/share UI #351

Closed
wants to merge 27 commits into from
Closed

Reorganize export/share UI #351

wants to merge 27 commits into from

Conversation

jfirebaugh
Copy link
Member

This is a followup to the map-ui changes, making better use of the new "Share" sidebar. It moves the more consumer-oriented options -- "Embeddable HTML" and "Image" -- alongside the "Link" option for sharing. The OSM XML "Export" option remains, but no longer needs the prominence of a top bar tab, so is moved to the sidebar alongside GPS Traces under a new "Data" section.

Two additional enhancements are included:

  • The oft-requested ability to include a marker in the permalink.
  • A shortlist of sources for bulk data downloads is provided when the selected area is too large for export.

View the changes on a test deployment

@tomhughes
Copy link
Member

Closing the share side bar leaves any marker and/or drag box visible on the map.

@tomhughes
Copy link
Member

Oh and the left side export side bar is not scrollable if it is larger than the screen. Actually I think I heard somebody say that is a general problem with the left sidebar and the new UI.

@lonvia
Copy link
Contributor

lonvia commented Jul 19, 2013

The marker gets lost when unchecking the 'include marker' check box, zooming to another region and switching the marker on again.

Also, the edit box in the embedding section resizes horizontally instead of vertically. I suspect that's not intentional.

@jfirebaugh
Copy link
Member Author

Thanks, those issues should be fixed.

@lonvia
Copy link
Contributor

lonvia commented Jul 19, 2013

The 'set custom dimensions' checkbox should also get cleared when the share panel is closed. Right now, there is a checked box but no image selector when the share panel is reopened.

@Zverik
Copy link
Contributor

Zverik commented Jul 20, 2013

This looks so great I want it on osm.org immediately :)

Can something be done with "Scale" field? Maybe replace it with zoom level, with a possible floating point (but defaulting to the current zoom level)? I doubt even 0.01% of site users understand what that number affects.

@Zverik
Copy link
Contributor

Zverik commented Jul 20, 2013

Short links are expanded to coordinates with 15 digits. I know it hardly matters, because links are short and expanded server-side, but still it's a bit confusing, since they are displayed in the location bar. Can those coordinates be cut at 6-7 digits, like for "long" links?

@jfirebaugh
Copy link
Member Author

@Zverik I agree on both points, but neither of those are regressions, so it would be better to file them as separate issues.

@Zverik
Copy link
Contributor

Zverik commented Jul 20, 2013

Done, #368 and #369.

@pnorman
Copy link
Contributor

pnorman commented Jul 24, 2013

The OSM XML "Export" option remains, but no longer needs the prominence of a top bar tab, so is moved to the sidebar alongside GPS Traces under a new "Data" section.

I can't actually find this, where is it?

@jfirebaugh
Copy link
Member Author

image

@pnorman
Copy link
Contributor

pnorman commented Jul 24, 2013

The prominence is okay, but it seems a bit odd when all the other links in the sidebar take you off the map to different pages. On the other hand, I can't think of anywhere else to place it that would be better.

@goldfndr
Copy link
Contributor

Shouldn't the Embeddable HTML's href's query string's parameters be separated by & instead of &?

@iandees
Copy link
Contributor

iandees commented Jul 29, 2013

Noticed a bug with marker location:

  1. New tab with http://mapui.apis.dev.openstreetmap.org/ (seems to happen only on first use)
  2. Click Share icon
  3. Enable "Include Marker"

Marker shows up at the center of the map before the right side bar made the map smaller.

screen shot 2013-07-29 at 3 34 50 pm

@jfirebaugh
Copy link
Member Author

@iandees Filed an upstream bug: Leaflet/Leaflet#1919.

@samanpwbb
Copy link
Member

I just pushed a change that makes map controls dark. These feel a little more prominent and also match the iD controls nicely.

We need to add active classes:

  • When panels are open, active on corresponding button
  • When map marker popup is open active on marker button (adds extra visual queue for how to add markers

I also added a .disabled rule in preparation for #396

@samanpwbb
Copy link
Member

Hi, can we please remove the blue link icons in the 'share' dialog. I understand the argument for them: you can right click the icon and then select 'copy link' from a list of options. However lets all step back for a second and think about how this looks to the majority of users.

There is a convention on the internet for small share links to follow urls in inputs. Here's an example:

screen shot 2013-07-30 at 3 50 54 pm

This is implemented with a little flash widget. Pretty convenient! Sadly, Flash is proprietary, ect. ect. we're not going to do this on osm.org and that's fair. Lets look at the share dialog in the osm sidebar:

screen shot 2013-07-30 at 3 49 11 pm

As a long-time Internet user, I expect that blue icon to function like the copy link on GitHub or countless other webistes. When I click it, I should get a friendly message that tells me the link has been copied to my clipboard, and I can go along on my way.

What actually happens when I want to copy the link to share my location by clicking the blue link? Three things:

  1. The page refreshes.
  2. The share dialog I once had open disappears.
  3. I have to re-open the share dialog, and make sure to avoid clicking that button that looks like it'd be really fun to click.

This is disorienting and confusing. This happens to me and I get suspicious of the OpenStreetMap website. Was that a bug?

We should remove these blue links. You can select the text in the input, right click, then copy. Or command+C.

@samanpwbb
Copy link
Member

Here's a proposal for a cleaner design:

share-redesign

  • Clearly separate the three different share techniques.
  • Separate marker visibility from the share sections.
  • Add a subtle background to bring emphasis to the link share option and help clarify that the section are not related to each other.
  • Use same typography conventions in the image share that we use elsewhere.

@iandees
Copy link
Contributor

iandees commented Jul 31, 2013

Re: The Scale number

Do we have enough information to compute the scale number based on a desired image size instead of prompting for a scale number (which is completely weird for the vast majority of users)?

Edit to add: Just noticed Zverik's comment on this.

@jfirebaugh
Copy link
Member Author

I've implemented a modified version of @samanpwbb's suggestions.

image

  • "Include marker" stays grouped with Link / HTML because it applies to these methods but not to Image.
  • Included "HTML" in the toggle.
  • "Link" and "Short Link" are actually link elements and you can do right-click -> copy link location. Therefore a separate link icon is unnecessary.

I'm leaving questions about scale to another PR. This one is almost two weeks in the making and there's already lots of good stuff in it -- let's get it merged.

@lonvia
Copy link
Contributor

lonvia commented Jul 31, 2013

There is still a glitch with "Set custom dimensions" and map movement: select and then unselect "set custom dimensions". Move map view somewhere else. Choose custom dimensions again and map will jump back to previous view.

@jfirebaugh
Copy link
Member Author

Fixed.

@pnorman
Copy link
Contributor

pnorman commented Jul 31, 2013

Do we have enough information to compute the scale number based on a desired image size instead of prompting for a scale number (which is completely weird for the vast majority of users)?

#368 has a more in-depth discussion of scale and the complicated stuff it's doing behind the scenes

@tomhughes
Copy link
Member

Is the change of colour of the heart icon on the donate button from black to white deliberate? or was it just changed accidentally when reversing the colour of the map controls?

"Image" and "Embeddable HTML" options move to share control.
Only XML export remains, and is now accessed via a sidebar
link rather than the tab bar.
@jfirebaugh
Copy link
Member Author

That was accidental, fixed.

@tomhughes
Copy link
Member

I think my main remaining concern with this is the removing of the ability to specify a separate location for the marker. That raises two issues:

  • Firstly it is removing functionality that is present on the existing site.
  • Secondly the resulting UI is a little odd because you can no longer move the map once you have asked for a marker. Well you can, but as soon as you release the mouse button it jumps back to the original location.

Before I merge this I'd like to hear what other people think about this...

@jfirebaugh
Copy link
Member Author

Yes, this ability was removed from the UI for the HTML embed. It was present in earlier revisions of this pull request and available for long/short link too, but it actually only worked with long link. I think that having a compact UI and simple, consistent options for embed, short, and long links is worth it. The behavior is also intended to address the complaints about not being able to easily create a centered marker at a precise location. Folks that need embed code with an off-center marker are still able to copy, paste, and tweak by hand.

@tomhughes
Copy link
Member

We discussed this at EWG and the general consensus seemed to be that loosing the functionality is not a problem.

Everybody felt that the second point is bug that we should try and address though - do you think there is anything we can do? Maybe we can just stop the map being moved once a marker has been requested? The marker would still be movable, which also moves the map centre.

@jfirebaugh
Copy link
Member Author

I think it's better if the map can still be panned and zoomed, but the marker stays fixed in the center. I've implemented that, but it requires some changes to Leaflet. I've submitted them upstream as well -- is it ok to have local patches temporarily?

@tomhughes
Copy link
Member

The UI certainly looks much nicer now - has @mourner given any indication as to whether the leaflet change (Leaflet/Leaflet#1939) is likely to get merged?

@tomhughes
Copy link
Member

Merged.

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

Successfully merging this pull request may close these issues.

None yet

8 participants