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

add the features api parameter which enables the list of features #4439

Merged
merged 6 commits into from
Oct 22, 2017

Conversation

ferdibiflator
Copy link
Contributor

@ferdibiflator ferdibiflator commented Oct 14, 2017

Fixes #4393

features api parameter is added for enabling the features in the list (, is the splitter)

Also the hash is being updated when the user toggles features

@bhousel
Copy link
Member

bhousel commented Oct 15, 2017

Thanks @ferdibiflator - this looks really good! I'll check it out some more tomorrow, in the meantime can you also update the API.md doc "url parameters" sections to include the new option?

@ferdibiflator
Copy link
Contributor Author

@bhousel I found a problem with boundaries feature. At the moment this one may not be enabled through the url parameter features.
Because the boundaries is disabled within the /index.html:

id = iD.Context()
            .assetPath('dist/');

id.features().disable('boundaries');

id.ui()(document.getElementById('id-container'), function() {

I suggest to disable this one in accordance with some logic within the modules/renderer/features.js. But I don't know in which cases the boundaries should be disabled.

@ferdibiflator
Copy link
Contributor Author

Hello @bhousel. Any news?

@bhousel
Copy link
Member

bhousel commented Oct 18, 2017

Any news?

This looks good, and I'll merge it any day now, thanks for being patient.
I'm merging about 1 PR/day but for this one I would like to check out the branch locally and test.

@bhousel
Copy link
Member

bhousel commented Oct 22, 2017

Hey @ferdibiflator I tried it out and I think I'm going to change it so that the url querystring contains the hidden features, instead of the shown features.. It's just a little less wordy that way. What you did is great, so I'm just going to merge as-is and make the change in a followup commit 👍

@bhousel bhousel merged commit 3332847 into openstreetmap:master Oct 22, 2017
@bhousel
Copy link
Member

bhousel commented Oct 23, 2017

btw @ferdibiflator re this:

I found a problem with boundaries feature. At the moment this one may not be enabled through the url parameter features

I worked around this in 30ff683 by changing the code in index.html to only disable boundaries if there is no disable_features list in the querystring. This means that if somebody really wants the boundaries enabled, they can add an empty param like &disable_features= to the url.

I mostly consider index.html as sample code that anyone is free to modify if they are reusing iD for other things.

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.

2 participants