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

Feature/Virtual Page Action Executor #1649

Merged
merged 23 commits into from
Jul 26, 2021
Merged

Feature/Virtual Page Action Executor #1649

merged 23 commits into from
Jul 26, 2021

Conversation

ibelar
Copy link
Contributor

@ibelar ibelar commented Jul 14, 2021

Modal executors can be problematic sometimes on Mobile devices. This PR will added options to use a VirtualPage Executor instead which is more mobile-friendly.

In order to change executor type, simply register the new class for Model\UserAction

$factory = $app->getExecutorFactory();
$factory->registerTypeExecutor($factory::STEP_EXECUTOR, [VpExecutor::class]);
  • Added a return button for returning to the base page;
  • Added a step list information with current step highlight in the list;
  • When action is executed (final step) it will return to base page;

Executor preview:

Executor in arguments step:
Screen Shot 2021-07-14 at 2 34 46 PM

Executor in fields step:
Screen Shot 2021-07-14 at 2 34 54 PM

Executor in preview step:
Screen Shot 2021-07-14 at 2 35 02 PM

@ibelar ibelar marked this pull request as draft July 14, 2021 19:14
@ibelar ibelar marked this pull request as ready for review July 14, 2021 21:59
@ibelar ibelar added the RTM label Jul 14, 2021
@mvorisek mvorisek removed their request for review July 19, 2021 13:22
src/CardDeck.php Outdated Show resolved Hide resolved
Copy link
Member

@mvorisek mvorisek left a comment

Choose a reason for hiding this comment

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

I checked only the URL building, which seems ok now

@@ -103,6 +103,7 @@ protected function init(): void

if ($this->paginator !== false) {
$this->addPaginator();
$this->stickyGet($this->paginator->name);
Copy link
Member

Choose a reason for hiding this comment

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

if removed, does Behat testing fails?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there is no behat test for Card deck with either VpExecutor or new PanelExecutor. This is because the default executor register in ExecutorFactory is ModalExecutor. I can added tests but we need a way to be able to switch executor types dynamically. Any idea? Should we switch base on GET argument in init-app?

Copy link
Member

Choose a reason for hiding this comment

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

I meant if sticky args in Card Deck are tested.

with either VpExecutor or new PanelExecutor...

I belive this does not need extra cross testing

@ibelar ibelar merged commit 6441241 into develop Jul 26, 2021
@ibelar ibelar deleted the feature/vp-executor branch July 26, 2021 14:39
@mvorisek mvorisek restored the feature/vp-executor branch October 4, 2021 00:04
@mvorisek mvorisek deleted the feature/vp-executor branch October 4, 2021 00:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants