-
Notifications
You must be signed in to change notification settings - Fork 6
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
[DO NOT MERGE YET] Remove slimmer/static dependency #1890
Open
KludgeKML
wants to merge
5
commits into
main
Choose a base branch
from
no-slimmer
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
e2186cf
to
5a0885d
Compare
5a0885d
to
f3cadd2
Compare
662a4e2
to
e3095fa
Compare
e3095fa
to
338cd97
Compare
338cd97
to
1730ba6
Compare
39e5915
to
29591e6
Compare
f148e23
to
91cf9d9
Compare
- TODO about choosing account layout will be addressed in a later commit, for the moment just remove all slimmer calls. We intend to replace this with a custom layout file for accounts.
- Update layout to use layout_for_public component directly - Update JS to include JS support for layout components - Update SCSS file to build component support CSS We won't get this from static any more - Add exclude_css_from_static = false to configuration - Layout does not include account-specific code, and has a section explicitly adding gem_component CSS for the components added by layout_for_public (since these can't be included automatically). Once added, we call render_component_stylesheets as part of the content_for :head, which is yielded to in the layout. - Layout removes the metadata_description section, this will be handled in the following commit. - rendering-application (added by slimmer) meta-tag recreated here.
- This was previously handled as a content_for :meta_description in the layout, but it was only used by this template. Since we now have a content_for :head in the layout, use it here to append to that block before it's sent to the layout_for_public component. - Add a test for this, since it wasn't being checked for and it is useful to confirm this refactor. Audit Trail: https://github.com/alphagov/email-alert-frontend/blob/bc3ffdcd72f09708635558350493d5ef9549a018/app/views/layouts/application.html.erb#L34-L36 https://github.com/alphagov/email-alert-frontend/blob/bc3ffdcd72f09708635558350493d5ef9549a018/app/views/subscription_authentication/expired.html.erb#L2
- this replicates/replaces the code from: https://github.com/alphagov/static/blob/f88f6e405c63e384411bdc7f25ec926b63d29f06/app/views/root/gem_layout_account_manager.html.erb https://github.com/alphagov/static/blob/f88f6e405c63e384411bdc7f25ec926b63d29f06/app/views/root/_gem_base.html.erb ...and moves this configuration to the only app that uses it. - add rendering-application meta tag (previously supplied by slimmer)
- Add JS used by global banners, and banner dependencies - Add global/emergency banner support in layouts - Add initializer for emergency banner REDIS support - Mock redis client in all tests (we're not explicitly testing emergency banner support here, since we assume that's tested adequately in the gem).
91cf9d9
to
1cfc9cd
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Removes the need to call out to static via slimmer for layouts.
https://trello.com/c/O6JhukLu/245-remove-slimmer-email-alert-frontend
Practical differences in rendered pages:
http-equiv="Content-Type"
element, as that is now obsolete (<meta charset="utf-8">
) covers it.