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

Support i18n and nested template in ScriptTemplateView #1304

Closed
wants to merge 1 commit into from

Conversation

sdeleuze
Copy link
Contributor

@sdeleuze sdeleuze commented Jan 24, 2017

This commit changes the 3rd parameter passed to the rendering
function from String url to RenderingContext renderingContext.

RenderingContext contains 4 properties:

  • ApplicationContext applicationContext
  • Locale locale
  • Function<String, String> templateLoader
  • String url

Issue: SPR-15064

@sdeleuze
Copy link
Contributor Author

@pgrimard Feel free to have a look and say me if that would fit with your need.

@pgrimard
Copy link

I like the approach you've taken with RenderingContext. The template loader is a nice way to keep templates modular and composable within the render function. With the introduction of rendering context, the template variable seems a little less important. I see it as a convenience if the render function doesn't need the added functionality of the rendering context, but ultimately the same template could be retrieved simply using the rendering context. So replacing the template argument with the rendering context could be an option, although a breaking change.

@sdeleuze
Copy link
Contributor Author

Indeed it could have been an option initially, but that would be too much breaking now, so I think I prefer the solution where we provide the content of the current template, and provide the RenderingContext facility for more advanced use cases (i18n, nested templates, etc.).

@pgrimard
Copy link

Makes sense 👍

This commit changes the 3rd parameter passed to the rendering
function from String url to RenderingContext renderingContext.

RenderingContext contains 4 properties:
 - ApplicationContext applicationContext
 - Locale locale
 - Function<String, String> templateLoader
 - String url

Issue: SPR-15064
@sdeleuze
Copy link
Contributor Author

Merged via this commit.

@sdeleuze sdeleuze closed this Jan 25, 2017
@pgrimard
Copy link

This will be release as part of Spring 5?

@sdeleuze
Copy link
Contributor Author

Yes, available in a few minutes in Spring Framework 5 snapshots for both Spring MVC and Spring Web Reactive, and in Spring Framework 5 RC1 and RELEASE.

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