Skip to content

Conversation

@duairc
Copy link
Contributor

@duairc duairc commented Mar 27, 2014

Generate the login/logout link server-side instead of with javascript.

I added a template variable loggedinuser :: String which is set to the name of the logged in user if there is a user logged in, and is unset otherwise.

This means the login/logout links in the userbox can be generated server-side now, and the javascript previously used is redundant. I changed the userbox code and removed the j

This techically changes the public API, as filledPageTemplate now takes a Maybe User argument which it didn't take previously. Probably the version in gitit.cabal should be

I added a template variable `loggedinuser :: String` which is set to the name of the logged in user if there is a user logged in, and is unset otherwise.

This means the login/logout links in the userbox can be generated server-side now, and the javascript previously used is redundant. I changed the userbox code and removed the javascript accordingly.

This techically changes the public API, as `filledPageTemplate` now takes a `Maybe User` argument which it didn't take previously. Probably the version in gitit.cabal should be bumped if this change is to be merged.
@jgm
Copy link
Owner

jgm commented Apr 2, 2014

If I recall, my reason for generating the link with javascript was to make it possible to cache the pages. If it's generated server-side, then the cached page will contain someone else's login/logout link.

@duairc
Copy link
Contributor Author

duairc commented Apr 3, 2014

Okay, that makes sense. Do you think if I did a commit that still added this template variable (and consequently changed the interface of filledPageTemplate), but left the Javascript stuff alone, would you accept that?

@duairc
Copy link
Contributor Author

duairc commented Apr 3, 2014

Okay, I went ahead and did this, the new pull request is in #414 now.

@duairc duairc closed this Apr 3, 2014
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