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

Fix FileLayout to work in non Web context, like Cli, Api #38585

Merged
merged 5 commits into from
Jan 22, 2023

Conversation

Fedik
Copy link
Member

@Fedik Fedik commented Aug 24, 2022

Fix for stuff like #38222 and #38214 .

Summary of Changes

FileLayout is fail to be render in non Web context, because it does not have a template, obviously.
I changed FileLayout class, so it will ignore the template in non Web context if it not provided within the options.

Testing Instructions

Create file layouts/potato.php with content:

<?php
echo 'Layout works!';

Add layout rendering:

$symfonyStyle->success(\Joomla\CMS\Layout\LayoutHelper::render('potato'));

Around here

$symfonyStyle->success('Cache cleaned');

Then from console run php cli/joomla.php cache:clean

Actual result BEFORE applying this Pull Request

an error: getTemplate undefined

Expected result AFTER applying this Pull Request

No error, you should see extra message from potato layout

Documentation Changes Required

Probably. FileLayout support template option, that should be a template name.

@Fedik Fedik changed the title Fix FileLayout to work in non Web context Fix FileLayout to work in non Web context, like Cli, Api Aug 24, 2022
@brianteeman
Copy link
Contributor

I have tested this item ✅ successfully on eb5a5bf


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38585.

1 similar comment
@dgrammatiko
Copy link
Contributor

I have tested this item ✅ successfully on eb5a5bf


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38585.

@brianteeman
Copy link
Contributor

I have tested this item 🔴 unsuccessfully on eb5a5bf


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38585.

@richard67
Copy link
Member

@brianteeman Why the unsuccessful test? What hasn't worked?

@viocassel
Copy link
Contributor

I have tested this item ✅ successfully on eb5a5bf


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38585.

@brianteeman
Copy link
Contributor

I have not tested this item.

marking as not tested. Comment to error on this PR #38650 (comment) suggests thatb this PR is not complete


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38585.

@Fedik
Copy link
Member Author

Fedik commented Jan 22, 2023

@brianteeman that error do not related to FileLayout class which in current PR,
that about PluginHelper::getLayoutPath() that is separated thing, the PR for it is there #39550

@richard67
Copy link
Member

I see the remaining issue is covered by another PR #39550 , so this PR here seems to be complete. See comment here: #38650 (comment) .

@richard67
Copy link
Member

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38585.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jan 22, 2023
@fancyFranci fancyFranci merged commit 562c285 into joomla:4.2-dev Jan 22, 2023
@fancyFranci fancyFranci added this to the Joomla! 4.2.7 milestone Jan 22, 2023
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jan 22, 2023
@fancyFranci
Copy link
Contributor

Thank you for your work, Fedik!

@Fedik Fedik deleted the fix-layout-cli branch January 22, 2023 16:28
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.

7 participants