Skip to content

Render the sign-in/sign-out button in the Dashboard header using Ajax#4723

Merged
philbogle merged 7 commits into
stagingfrom
ajax_sign_in
Oct 28, 2015
Merged

Render the sign-in/sign-out button in the Dashboard header using Ajax#4723
philbogle merged 7 commits into
stagingfrom
ajax_sign_in

Conversation

@philbogle

Copy link
Copy Markdown
Contributor

We now render the user header containing the sign-in/sign-out button in the Dashboard header using Ajax. This is the same Ajax action used for rendering the button in Pegasus. Ajax rendering allows the user header to be rendered correctly for signed in users even in publicly cached pages.

Testing

  • Visually compare positioning and rendering of Signin/Signout button on my machine vs. staging across a number dashboard pages.
  • Run UI Tests against adhoc machine

@philbogle

Copy link
Copy Markdown
Contributor Author

@laurel This is the change to render the user header using ajax as we discussed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we usually use $ for jQuery

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@laurelfan

Copy link
Copy Markdown
Contributor

LGTM except for minor style comments

and it looks like some tests for the user menu should be changed to test ApiController#user_menu instead of HomeController#index

@philbogle

Copy link
Copy Markdown
Contributor Author

Laurel, please see whether the stylesheet changes I made look good.

I will check with you in person about the tests to update.

@philbogle

Copy link
Copy Markdown
Contributor Author

I moved the appropriate tests to api_controller_test. I also added tests to make sure the menu renders the correct "sign in" and "sign out" links based on the sign in state.

@philbogle

Copy link
Copy Markdown
Contributor Author

I added back in the hack to fixed levels configured with an invalid base url (host name but no leading '//') so now all levels are rendering as expected.

@laurelfan

Copy link
Copy Markdown
Contributor

LGTM. The UI test for the signin button that I just added may be relevant to this change:

https://github.com/code-dot-org/code-dot-org/blob/staging/dashboard/test/ui/features/signingIn.feature

I made it chrome only because other browsers were having problems with state between scenarios (the browser's cache I think?). It would be interesting to see if these things magically go away when it is ajaxified. It currently works on Chrome and Firefox.

philbogle added a commit that referenced this pull request Oct 28, 2015
Render the sign-in/sign-out button in the Dashboard header using Ajax
@philbogle philbogle merged commit 9ac35c2 into staging Oct 28, 2015
@philbogle

Copy link
Copy Markdown
Contributor Author

Eyes tests showed some layout issue and cross-test state contamination, so I'm going to roll this back for investigation.

deploy-code-org added a commit that referenced this pull request Oct 29, 2015
16bc449 Merge pull request #4903 from code-dot-org/only_setup_local_pegasus_db (philbogle)
7460930 Merge pull request #4911 from code-dot-org/remerge-project-expect-channel (Josh Lory)
4907658 Merge pull request #4915 from code-dot-org/revert-4723-ajax_sign_in (philbogle)
c266d62 Revert "Render the sign-in/sign-out button in the Dashboard header using Ajax" (philbogle)
0dc2fb9 Header (and 'Remix' button) doesn't show up on mobile [ci skip] (Josh Lory)
7317599 Merge pull request #4913 from code-dot-org/all-responses (Laurel)
9ac35c2 Merge pull request #4723 from code-dot-org/ajax_sign_in (philbogle)
deploy-code-org added a commit that referenced this pull request Oct 29, 2015
224dc67 Merge pull request #4786 from code-dot-org/pulse-item (Brad Buchanan)
d619792 Automatically built. (Continuous Integration)
8403a32 Merge pull request #4920 from code-dot-org/testfixes (Elijah Hamovitz)
0ff348c Temporarily removing funOMeter test (Elijah Hamovitz)
b457e24 Merge pull request #4916 from code-dot-org/ie10tests (Bjvanminnen)
5b89e35 more IE10 (Brent Van Minnen)
1876a7f o Merge branch 'staging' of github.com:code-dot-org/code-dot-org into staging (Trevor Berg)
541fdc5 Fix issues with JSONFileDataAdapter (Trevor Berg)
0f13914 iOS requires directly-bound click events and 'cursor: pointer' style for click events to work (Elijah Hamovitz)
5b6b40f Feature-check SVG filter support before creating/applying a filter. (Brad Buchanan)
656f7c2 Merge pull request #4877 from code-dot-org/bundle_install_frontends (Will Jordan)
b7be14d Automatically built. (Continuous Integration)
16bc449 Merge pull request #4903 from code-dot-org/only_setup_local_pegasus_db (philbogle)
7460930 Merge pull request #4911 from code-dot-org/remerge-project-expect-channel (Josh Lory)
f53452b start running applab tests on IE 10 (Brent Van Minnen)
4907658 Merge pull request #4915 from code-dot-org/revert-4723-ajax_sign_in (philbogle)
c266d62 Revert "Render the sign-in/sign-out button in the Dashboard header using Ajax" (philbogle)
0dc2fb9 Header (and 'Remix' button) doesn't show up on mobile [ci skip] (Josh Lory)
7317599 Merge pull request #4913 from code-dot-org/all-responses (Laurel)
3130e6c Merge remote-tracking branch 'origin/staging' into pulse-item (Brad Buchanan)
e28f355 Deduplicate functions and remove last SMIL TODO comment. (Brad Buchanan)
9ac35c2 Merge pull request #4723 from code-dot-org/ajax_sign_in (philbogle)
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