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

Feature/issue 44 config via url #53

Open
wants to merge 4 commits into
base: v2-alpha
Choose a base branch
from

Conversation

ninovanhooff
Copy link

@ninovanhooff ninovanhooff commented Jun 4, 2019

Issue: 44

Description of changes: Whenever settings change the url is updated with a base64 encoded version of the entire settings object. The url can be shared and bookmarked. When loading the settings controller, the url is checked for the settings query param and it is decoded when found. This pre-populates the settings modal

Areas for improvement

  • instead of serializing the entire settings, only store the changes with respect to the defaults. This might help a bit with reducing the url length. But not sure whether long urls instead of very long urls make a meaningful difference
  • a request was made for storing it in a more human-readable form instead of base64. Don't see the need myself, though
  • users still are presented with the settings modal. In case no auth is required, it woiuld be nice if the modal could be skipped. My angular knowledge is not sufficient to get this working though.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@ninovanhooff
Copy link
Author

ninovanhooff commented Jun 4, 2019

My attempt at skipping the modal (not working):
Bottom of explorer.js

if (qs("skip_settings")){
        const service = angular.element('*[ng-app]').injector().get("SharedService")
        const settings = service["getSettings"].call()
        DEBUG.log("using settings:", settings);
        service["changeSettings"].call(settings)
    } else {
        $('#SettingsModal').modal({ keyboard: true, backdrop: 'static' });
    }

It seems that functions can be called this way, but the argument to call() seems to be ignored and the retrn value for getSettings is similarly undefined.

@ninovanhooff
Copy link
Author

Open question: to what extent does the browser interfere with prefilling the fields in case the user has used the page before and the browser might try to replace form field values?

@ninovanhooff ninovanhooff mentioned this pull request Jun 4, 2019
@ninovanhooff
Copy link
Author

@john-aws Any feedback regarding the bullet points I mentioned or the PR in general?

@john-aws
Copy link
Contributor

@ninovanhooff Apologies for the delay and thanks for your work here. I will try to find time this week to look at the changes.

@rfelgent
Copy link

hi @john-aws ,

is this PR still in progress ?

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.

3 participants