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.

Sign up for GitHub

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

Jump to bottom

Added $templateFile to UI\Control #206

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Added $templateFile to UI\Control #206

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9f0f3ed
Added $templateFile to UI\Control
Dec 6, 2018
08cd250
Fix coding standards
Dec 10, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 27 additions & 1 deletion src/Application/UI/Control.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ abstract class Control extends Component implements IRenderable
/** @var ITemplateFactory */
private $templateFactory;

/** @var string|null */
private $templateFile;

/** @var ITemplate */
private $template;

Expand All @@ -42,6 +45,23 @@ final public function setTemplateFactory(ITemplateFactory $templateFactory)
}


final public function setTemplateFile(string $templateFile = null)
{
$this->templateFile = $templateFile;

if ($this->template !== null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Conditional setter? Never seen that before. Should throw exception or create template if not already created.
I understand why it's here, but it's a just a very specific usecase.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if you got it right. Whole idea of this PR is to be able setup file without creating template. If we throw exception or create template there is no PR needed any more, cause it will work just like shortcut for $control->getTemplate()->setFile().

But I thought about it if I should add this condition. Because it's just for specific usecase as you said when user try to set file after template was created to prevent "WTF?" situations and let setter work like user expect.

Copy link
Contributor

@mabar mabar Dec 10, 2018 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I said, I understand. But imagine you don't know that code, just api. It looks exactly like a shortcut, but acts absolutely different.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't, I'm not saying you're not right, but I think user don't care about it, he just want to set template file/view and this did that without unnecessary loads.

And I really hate setting template in $onAnchor event. Or do you know way how to do it without event in control factory or in createComponent<name>() method?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you could configure template file through it's factory definition

fooControlFactory:
  implement IFooControlFactory
  setup:
    - getTemplate()->setFile('/path/to/file')

But I never used that approach. I have ITemplate with setView method and to change path to these views I only need change pattern in configuration.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will end with InvalidStateException or new template instance. Anyway I don't like to setup file template in config, there are situations where you want to setup view by condition, or you have one control with factory and want to setup many different view for actual page.

$this->template->setFile($templateFile);
}
return $this;
}


final public function getTemplateFile(): ?string
{
return $this->templateFile;
}


final public function getTemplate(): ITemplate
{
if ($this->template === null) {
Expand All @@ -54,7 +74,13 @@ final public function getTemplate(): ITemplate
protected function createTemplate(): ITemplate
{
$templateFactory = $this->templateFactory ?: $this->getPresenter()->getTemplateFactory();
return $templateFactory->createTemplate($this);
$template = $templateFactory->createTemplate($this);

if ($this->templateFile !== null) {
$template->setFile($this->templateFile);
}

return $template;
}


Expand Down