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

UI: Refactor duplicate template rendering code #424

Merged
merged 3 commits into from
Aug 9, 2021
Merged

UI: Refactor duplicate template rendering code #424

merged 3 commits into from
Aug 9, 2021

Conversation

ato
Copy link
Collaborator

@ato ato commented Aug 1, 2021

While trying to add a new feature to the user interface I discovered that each of the Resource classes duplicates the same code for rendering Freemarker templates. Rather than creating yet another copy of it I'd prefer to first refactor and eliminate the duplication.

This replaces the various copies of the Freemarker configuration with one shared instance attached to the application object. It also introduces a render() helper in BaseResource that sets up the common view parameters and returns a HTML Representation.

I've removed the calls to representation.setCharacterSet(UTF_8) as WriterRepresentation's parent's constructor ends up calling that anyway.

Note that EngineResource uses DefaultObjectWrapper while the other pages use BeansWrapper, so I added an overload to render() to preserve that difference. It'd be nice if they used the same configuration but I'll leave tackling that for future.

ato added 3 commits August 1, 2021 15:23
So we don't need to configure it separately in every resource class that
uses HTML templates.
We remove the calls to setCharacterSet(UTF_8) since
WriterRepresentation's constructor does that anyway.
Each direct subclass of BaseResource defines an identical getEngine()
method so let's pull it up to BaseResource. We can also don't need the
type cast anymore as BaseResource.getApplication() does it for us.
@ato ato merged commit cd30fbc into master Aug 9, 2021
@ato ato deleted the ui-cleanup branch August 9, 2021 05:41
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.

1 participant