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

More reactive, tested Masthead. #9822

Merged
merged 11 commits into from
May 28, 2020

Conversation

jmchilton
Copy link
Member

... also less broken since there are some bug fixes in here.

  • Eliminate another layer of Backbone.View (the one previously in layout/masthead.js) and put the Masthead component directly in the page.
  • Fix onbefeoreunload handling broken in [WIP] Porting Masthead to Vue.js #9071 I think.
  • Remove more underscore and jQuery usage.
  • Make config handling around brand config reactive and implement a unit test.
  • Eliminate hackery around loading webhooks in different ways in different context, just load them when component is created. Place them in a data entry so the underlying menu structure can change reactively and these will remain.
  • Add unit test for loading masthead webhooks.
  • Make actual entries in the masthead more reactive - previously it would just be generated once in masthead.js

Builds on #9811 (which converted layout/menu.js to use a POJO instead of Backbone and #9810 which contains other masthead fixes.

@jmchilton jmchilton changed the title [WIP] More reactive, tested Masthead. More reactive, tested Masthead. May 27, 2020
@galaxybot galaxybot added this to the 20.09 milestone May 27, 2020
@dannon
Copy link
Member

dannon commented May 28, 2020

Working well for me, thanks.

@dannon dannon merged commit aeba343 into galaxyproject:dev May 28, 2020
@galaxybot
Copy link
Contributor

This PR was merged without a 'kind/' tag, please correct.

@jmchilton
Copy link
Member Author

Thanks for the quick merge of a big change @dannon, much appreciated.

@nsoranzo nsoranzo deleted the masthead_improvements_1 branch May 28, 2020 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants