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

Bypass application layout rendering for SessionsController #333

Merged
merged 4 commits into from Aug 24, 2016

Conversation

mariusbutuc
Copy link
Contributor

@mariusbutuc mariusbutuc commented Aug 23, 2016

The SessionsController inherits rendering using the application layout.

By default, render with no layout at all.

/review @kevinhughes27 @alexcoco @swalkinshaw
/cc @nickhoffman

@alexcoco
Copy link
Member

The new action should be rendered without a layout, why not update the session concern and set layout false for that action

@nickhoffman
Copy link
Contributor

@alexcoco, what's the benefit of specifying no layout for the new action only?

@alexcoco
Copy link
Member

The difference is that if someone inherits this controller then you're disabling it for all the actions even the ones that they add. I rather it be more explicit.

@nickhoffman
Copy link
Contributor

I usually err on the side of explicitness too. However, I figured that inheriting from this controller and adding actions is uncommon, and when it does occur, the application's default layout wouldn't be wanted. But if your experience says otherwise, 👍

@kevinhughes27
Copy link
Contributor

I would prefer the explicitness as well

@mariusbutuc
Copy link
Contributor Author

Disabling the layout rendering moved into the concern, and it is now scoped to the #new action only, as of 069d708.

@kevinhughes27 @alexcoco @nickhoffman please have another look.

@nickhoffman
Copy link
Contributor

Code LGTM. 🚢 once you bump the version and CI is green.

@kevinhughes27
Copy link
Contributor

👍

@alexcoco
Copy link
Member

👍

@mariusbutuc mariusbutuc merged commit 076a296 into master Aug 24, 2016
@mariusbutuc mariusbutuc deleted the no-layout-for-sessions branch August 24, 2016 15:55
@mariusbutuc mariusbutuc temporarily deployed to rubygems August 24, 2016 16:11 Inactive
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.

None yet

4 participants