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

Allow more control over the application layout #1767

Merged
merged 3 commits into from
Feb 8, 2024

Conversation

stefanvermaas
Copy link
Contributor

TLDR: The PR gives an app developer more granular control over what layout to render when using the shopify_app gem to build an embedded Shopify app.

When running a Shopify app in embedded mode, every controller that requires an authenticated session will use the embedded app layout by default. This is enforced by the ShopifyApp::EmbeddedApp module.

As a default, this is great. It helps the developer to make fewer choices when they start to develop their new Shopify app. However, sometimes one wants to run an embedded app in non-embedded mode.

Some examples of running a normally embedded app in non-embedded mode:

  • When the remote environment is a CI;
  • When the remote environment is a preview/PR app;
  • When the developer wants to run the app in a non-embedded mode for testing.

The static implementation of the embedded app layout causes duplication and maintainability issues when running in both modes.

This PR introduces a more flexible system whereby the developer has more control over what layout is being rendered during the process and allows the developer to ignore the embedded app layout.

Instead, the developer can implement the embedded app layout (as provided by the gem) once in the application.html.erb and set the

Now, when running the app both in embedded and non-embedded mode, the developer only has to maintain a single layout file for all controllers in its app.

Reviewer's guide to testing

To use the same application layout for every application controller, a developer can now overwrite the #use_embedded_app_layout? method.

class ApplicationController
  # Ensures every controller is using the standard layouts/application.html.erb layout.
  # 
  # @return [true, false]
  def use_embedded_app_layout?
    false
  end
end

Checklist

Before submitting the PR, please consider if any of the following are needed:

  • Update CHANGELOG.md if the changes would impact users
  • Update README.md, if appropriate.
  • Update any relevant pages in /docs, if necessary
  • For security fixes, the Disclosure Policy must be followed.

@stefanvermaas stefanvermaas requested a review from a team as a code owner December 16, 2023 05:00
@matteodepalo
Copy link
Contributor

Hi @stefanvermaas, thank you so much for your contribution! Would it be possible to add something to the documentation that mentions this new option?

@stefanvermaas
Copy link
Contributor Author

Would it be possible to add something to the documentation that mentions this new option?

Yes, of course! I refrained from adding documentation initially as I wanted to see if there was an appetite for the Shopify team for this proposed change. I'll update the PR with accompanying documentation.

@lizkenyon lizkenyon added the Waiting for Response Need more information before we can provide more assistance label Jan 3, 2024
@stefanvermaas
Copy link
Contributor Author

@matteodepalo Sorry for the delay, I've just added the documentation. Please let me know if any changes are required.

@github-actions github-actions bot removed the Waiting for Response Need more information before we can provide more assistance label Feb 6, 2024
@matteodepalo
Copy link
Contributor

@stefanvermaas looks good! Could you rebase the branch?

When running a Shopify app in embedded mode, every controller that
requires an authenticated session will use the embedded app layout
by default.

As a default, this is great. It helps the developer to make less
choices when they start to develop their new Shopify app. However,
sometimes one wants to run an embedded app in non-embedded mode.

Some examples of running a normally embedded app in non-embedded mode:
- When the remote environment is a CI;
- When the remote environment is a preview/PR app;
- When the developer wants to run the app as a non-embedded mode for testing.

The rather static implementation of the embedded app layout causes
duplication and maintainability issues when running in both modes.

This commit introduces a more flexible system whereby the deverloper
has more control over what layout is being rendered during the process
and it allows the developer to ignore the embedded app layout.

Instead, the developer can implement the embedded app layout (as
provided by the gem) once in the application.html.erb, and set the

Now, when running the app both in embedded and non-embedded mode,
the developer only has to maintain a single layout file for all
controllers in its app.
@matteodepalo matteodepalo merged commit 8fc434a into Shopify:main Feb 8, 2024
6 checks passed
@stefanvermaas
Copy link
Contributor Author

Thanks @matteodepalo!

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.

3 participants