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 configurable parameter `update_browser_title` #904

Merged
merged 1 commit into from Jul 11, 2017

Conversation

Projects
None yet
4 participants
@lukaw3d
Contributor

lukaw3d commented Jul 3, 2017

No description provided.

@hadalin

This comment has been minimized.

hadalin commented Jul 10, 2017

Are PRs watched?

hello

@nathandunn

This comment has been minimized.

Contributor

nathandunn commented Jul 10, 2017

They are, but we have a lot of other stuff going on. This is a good idea, though.

To that end some recommendations:

  • make the property name non-camel case to match the other properties (update_browser_title)
  • put a option similar to the other properties in index.html so that it can be turned off / on easily
  • probably a good idea to coerce the boolean in Browser.js (too lazy to find the exact line number):
     dojo.forEach( ['show_tracklist','show_nav','show_overview','show_menu', 'show_fullviewlink', 'show_tracklabels'], function(v) {
          this.config[v] = this._coerceBoolean( this.config[v] );
   },this);
@lukaw3d

This comment has been minimized.

Contributor

lukaw3d commented Jul 11, 2017

Super 😄

Updated with the recommendations
(update_browser_title now follows how show_menu appears in code, instead of updateBrowserURL)

I'm not sure if it should be added into compat_121.html as well

@nathandunn

This comment has been minimized.

Contributor

nathandunn commented Jul 11, 2017

tested and looks great. Thanks for the PR.

@nathandunn nathandunn merged commit 7c10ece into GMOD:master Jul 11, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

lukaw3d added a commit to genialis/jbrowse-built that referenced this pull request Jul 11, 2017

@lukaw3d lukaw3d changed the title from Add configurable parameter `updateBrowserTitle` to Add configurable parameter `update_browser_title` Jul 11, 2017

lukaw3d added a commit to genialis/jbrowse-built that referenced this pull request Jul 11, 2017

@rbuels rbuels added this to the 1.12.4 milestone Feb 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment