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 nested accordion open #1863

Merged
merged 9 commits into from
Sep 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 7 additions & 18 deletions demos/interactive/accordion-nested.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,17 @@
namespace Atk4\Ui\Demos;

use Atk4\Ui\Accordion;
use Atk4\Ui\Button;
use Atk4\Ui\Form;
use Atk4\Ui\Header;
use Atk4\Ui\LoremIpsum;
use Atk4\Ui\Message;
use Atk4\Ui\View;

/** @var \Atk4\Ui\App $app */
require_once __DIR__ . '/../init-app.php';

/*
Button::addTo($app, ['View Form input split in Accordion section', 'class.small right floated basic blue' => true, 'iconRight' => 'right arrow'])
->link(['accordion-in-form']);
View::addTo($app, ['ui' => 'clearing divider']);
*/

Header::addTo($app, ['Nested accordions']);

$addAccordionFunc = function ($view, $maxDepth = 2, $level = 0) use (&$addAccordionFunc) {
$addAccordionFunc = function ($view, $maxDepth, $level = 0) use (&$addAccordionFunc) {
$accordion = Accordion::addTo($view, ['type' => ['styled', 'fluid']]);

// static section
Expand All @@ -38,27 +30,24 @@
$i2 = $accordion->addSection('Dynamic Text', function ($v) use ($addAccordionFunc, $maxDepth, $level) {
Message::addTo($v, ['Every time you open this accordion item, you will see a different text', 'ui' => 'tiny message']);
LoremIpsum::addTo($v, ['size' => 2]);
if ($level < $maxDepth) {
$addAccordionFunc($v, $maxDepth, $level + 1);
}

$addAccordionFunc($v, $maxDepth, $level + 1);
});

// dynamic section - form view
$i3 = $accordion->addSection('Dynamic Form', function ($v) use ($addAccordionFunc, $maxDepth, $level) {
Message::addTo($v, ['Loading a form dynamically.', 'ui' => 'tiny message']);
$form = Form::addTo($v);
$form->addControl('Email');
$form->addControl('email');
$form->onSubmit(function (Form $form) {
return $form->success('Subscribed ' . $form->model->get('Email') . ' to newsletter.');
return $form->success('Subscribed ' . $form->model->get('email') . ' to newsletter.');
});

if ($level < $maxDepth) {
$addAccordionFunc($v, $maxDepth, $level + 1);
}
$addAccordionFunc($v, $maxDepth, $level + 1);
});

return $accordion;
};

// add accordion structure
$addAccordionFunc($app);
$addAccordionFunc($app, 4);
3 changes: 0 additions & 3 deletions phpstan.neon.dist
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,6 @@ parameters:
-
path: 'src/Form/Control/Checkbox.php'
message: '~^Access to an undefined property Atk4\\Ui\\Jquery::\$checked\.$~'
-
path: 'src/Form/Layout/Section/Accordion.php'
message: '~^Call to an undefined method Atk4\\Ui\\View::jsOpen\(\)\.$~'
-
path: 'src/JsVueService.php'
message: '~^Call to an undefined method Atk4\\Ui\\JsChain::createAtkVue\(\)\.$~'
Expand Down
2 changes: 1 addition & 1 deletion public/agileui.less
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
@tabletWidth: 768px;
@adminMenuHeight: 48px;
@atkFooterHeight : 50px;
@menuBorder: 1px solid rgba(255,255,255,0.10);
@menuBorder: 1px solid rgba(255,255,255,0.1);

@atkSlidePanelColor: @offWhite;

Expand Down
14 changes: 3 additions & 11 deletions src/Accordion.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ public function addSection($title, \Closure $callback = null, $icon = 'dropdown'
// if there is callback action, then use VirtualPage
if ($callback) {
$section->virtualPage = VirtualPage::addTo($section, ['ui' => '']);
$section->virtualPage->stickyGet('__atk-dyn-section', '1');
$section->virtualPage->set($callback);
}

Expand Down Expand Up @@ -152,22 +151,15 @@ public function getSectionIdx(AccordionSection $section)
return $idx;
}

/**
* Check if accordion section is dynamic.
*/
public function isDynamicSection(): bool
{
return isset($_GET['__atk-dyn-section']);
}

protected function renderView(): void
{
if ($this->type) {
$this->addClass($this->type);
}

// Only set Accordion in Top container. Otherwise Nested accordion won't work.
if (!$this->getClosestOwner($this, AccordionSection::class) && !$this->isDynamicSection()) {
Copy link
Member Author

Choose a reason for hiding this comment

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

@ibelar why isDynamicSection was introduced, it is still needed?

// initialize top accordion only, otherwise nested accordion won't work
// https://github.com/fomantic/Fomantic-UI/issues/254
if ($this->getClosestOwner(AccordionSection::class) === null) {
$this->js(true)->accordion($this->settings);
}

Expand Down
3 changes: 1 addition & 2 deletions src/Form/Control.php
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,7 @@ protected function renderTemplateToHtml(string $region = null): string
{
$output = parent::renderTemplateToHtml($region);

/** @var Form|null $form */
$form = $this->getClosestOwner($this, Form::class);
$form = $this->getClosestOwner(Form::class);

return $form !== null ? $form->fixOwningFormAttrInRenderedHtml($output) : $output;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Form/Layout/Section/Accordion.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ protected function init(): void
$jsError = [$form->js()->form('add prompt', $fieldName, $str)];

// if a form control is part of an accordion section, it will open that section.
$section = $form->getClosestOwner($form->getControl($fieldName), AccordionSection::class);
$section = $form->getControl($fieldName)->getClosestOwner(AccordionSection::class);
Copy link
Member Author

Choose a reason for hiding this comment

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

@mkrecek234 the getClosestOwner method was broken, does this fix #1728 too?

Copy link
Member Author

Choose a reason for hiding this comment

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

@mkrecek234 should be fixed when not nested

if ($section) {
$jsError[] = $section->getOwner()->jsOpen($section);
}
Expand Down
20 changes: 9 additions & 11 deletions src/Popup.php
Original file line number Diff line number Diff line change
Expand Up @@ -107,17 +107,15 @@ protected function init(): void
->addMoreInfo('owner', $this->getOwner());
}

if (($this->triggerBy instanceof Menu
|| $this->triggerBy instanceof MenuItem
|| $this->triggerBy instanceof Dropdown) && $this->triggerOn === null
) {
$this->triggerOn = 'hover';
}

if (
$this->triggerBy instanceof Button && $this->triggerOn === null
) {
$this->triggerOn = 'click';
if ($this->triggerOn === null) {
if ($this->triggerBy instanceof Menu
|| $this->triggerBy instanceof MenuItem
|| $this->triggerBy instanceof Dropdown
) {
$this->triggerOn = 'hover';
} elseif ($this->triggerBy instanceof Button) {
$this->triggerOn = 'click';
}
}

$this->popOptions = array_merge($this->popOptions, [
Expand Down
19 changes: 10 additions & 9 deletions src/View.php
Original file line number Diff line number Diff line change
Expand Up @@ -297,24 +297,25 @@ public function add($object, $region = null): AbstractView
}

/**
* Get objects closest owner which is instance of particular class.
* Get closest owner which is instance of particular class.
*
* If there are no such owner (or grand-owner etc.) object, then return.
* @template T of View
*
* Note: this is internal method, but should be public because other objects
* should be able to call it.
* @param class-string<T> $class
*
* @phpstan-return T|null
*/
public function getClosestOwner(self $object, string $class): ?self
public function getClosestOwner(string $class): ?self
{
if ($object->issetOwner()) {
if (!$this->issetOwner()) {
return null;
}

if ($object->getOwner() instanceof $class) {
return $object->getOwner();
if ($this->getOwner() instanceof $class) {
return $this->getOwner();
}

return $this->getClosestOwner($object->getOwner(), $class);
return $this->getOwner()->getClosestOwner($class);
}

// }}}
Expand Down
16 changes: 16 additions & 0 deletions tests-behat/accordion.feature
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,19 @@ Feature: Accordion
Given I am on "form/form-section-accordion.php"
Then I should see "Email"
Then I fill in "email" with "[email protected]"

Scenario: Nested Accordion
Given I am on "interactive/accordion-nested.php"
Then I click using selector "xpath((//div[text()='Static Text'])[1])"
Then I click using selector "xpath((//div[text()='Static Text'])[1])"
Then I click using selector "xpath((//div[text()='Static Text'])[1])"
Then I click using selector "xpath((//div[text()='Static Text'])[2])"
Then I click using selector "xpath((//div[text()='Dynamic Text'])[3])"
Then I click using selector "xpath((//div[text()='Dynamic Text'])[3])"
Then I click using selector "xpath((//div[text()='Dynamic Text'])[3])"
Then I click using selector "xpath((//div[text()='Dynamic Form'])[4])"
Then I click using selector "xpath((//div[text()='Dynamic Form'])[4])"
Then I click using selector "xpath((//div[text()='Dynamic Form'])[4])"
Then I fill in "email" with "[email protected]"
When I press button "Save"
Then I should see "Subscribed [email protected] to newsletter."