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

IBX-6750/IBX-6754/IBX-6839: Implementation of smart/expert mode #1017

Merged
merged 17 commits into from
Dec 4, 2023
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
1 change: 1 addition & 0 deletions features/personas/AddLocation.feature
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Feature: Verify that an Editor with Content Type limitation on content/create po
And I log in as "Add" with password "Passw0rd-42"
And I go to "Content structure" in "Content" tab
And I navigate to content "NewArticle" of type "Article" in root
And I am using the DXP in "Expert" mode
When I switch to "Locations" tab in Content structure
And I add a new Location under "root/Destination"
Then there exists Content view Page for "root/Destination/NewArticle"
15 changes: 0 additions & 15 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -1750,11 +1750,6 @@ parameters:
count: 1
path: src/lib/Behat/BrowserContext/ContentViewContext.php

-
message: "#^Property Ibexa\\\\AdminUi\\\\Behat\\\\BrowserContext\\\\ContentViewContext\\:\\:\\$universalDiscoveryWidget is never read, only written\\.$#"
count: 1
path: src/lib/Behat/BrowserContext/ContentViewContext.php

-
message: "#^Method Ibexa\\\\AdminUi\\\\Behat\\\\BrowserContext\\\\LanguageContext\\:\\:deleteLanguageNamed\\(\\) has no return type specified\\.$#"
count: 1
Expand Down Expand Up @@ -3035,21 +3030,11 @@ parameters:
count: 1
path: src/lib/Behat/Page/ContentViewPage.php

-
message: "#^Property Ibexa\\\\AdminUi\\\\Behat\\\\Page\\\\ContentViewPage\\:\\:\\$contentUpdatePage is never read, only written\\.$#"
count: 1
path: src/lib/Behat/Page/ContentViewPage.php

-
message: "#^Property Ibexa\\\\AdminUi\\\\Behat\\\\Page\\\\ContentViewPage\\:\\:\\$route has no type specified\\.$#"
count: 1
path: src/lib/Behat/Page/ContentViewPage.php

-
message: "#^Property Ibexa\\\\AdminUi\\\\Behat\\\\Page\\\\ContentViewPage\\:\\:\\$userUpdatePage is never read, only written\\.$#"
count: 1
path: src/lib/Behat/Page/ContentViewPage.php

-
message: "#^Method Ibexa\\\\AdminUi\\\\Behat\\\\Page\\\\DashboardPage\\:\\:editDraft\\(\\) has no return type specified\\.$#"
count: 1
Expand Down
82 changes: 82 additions & 0 deletions src/bundle/Controller/User/UserModeController.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
<?php

/**
* @copyright Copyright (C) Ibexa AS. All rights reserved.
* @license For full copyright and license information view LICENSE file distributed with this source code.
*/
declare(strict_types=1);

namespace Ibexa\Bundle\AdminUi\Controller\User;

use Ibexa\AdminUi\Form\Data\User\UserModeChangeData;
use Ibexa\AdminUi\Form\Type\User\UserModeChangeType;
use Ibexa\AdminUi\UserSetting\UserMode;
use Ibexa\Contracts\AdminUi\Controller\Controller;
use Ibexa\User\UserSetting\UserSettingService;
use Symfony\Component\HttpFoundation\RedirectResponse;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;

final class UserModeController extends Controller
{
private const RETURN_URL_PARAM = 'returnUrl';

private UserSettingService $userSettingService;

public function __construct(UserSettingService $userSettingService)
{
$this->userSettingService = $userSettingService;
}

public function changeAction(Request $request, ?string $returnUrl): Response
{
$data = new UserModeChangeData();
$data->setMode($this->userSettingService->getUserSetting(UserMode::IDENTIFIER)->value === UserMode::EXPERT);

$form = $this->createForm(
UserModeChangeType::class,
$data,
[
'action' => $this->generateUrl(
'ibexa.user_mode.change',
[
self::RETURN_URL_PARAM => $returnUrl,
]
),
'method' => Request::METHOD_POST,
]
);

$form->handleRequest($request);
if ($form->isSubmitted() && $form->isValid()) {
$this->userSettingService->setUserSetting(
UserMode::IDENTIFIER,
$data->getMode() ? UserMode::EXPERT : UserMode::SMART
Copy link
Contributor

Choose a reason for hiding this comment

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

Why aren't we using UserMode::EXPERT and UserMode::SMART directly, instead of relying on a straight up boolean?

For me myself, it would make sense to use the same value as user settings. It would be also a lot easier to work with in case a third setting is introduced.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I commented on the same issue. Fully agree, user setting was prepared to accept more user modes in the future. We could maybe even make it extendable by developers, especially with suggestions like #1017 (comment) Data format should reflect values from the user setting.

);

return $this->createRedirectToReturnUrl($request);
}

return $this->render(
'@ibexadesign/ui/user_mode_form.html.twig',
[
'form' => $form->createView(),
]
);
}

private function createRedirectToReturnUrl(Request $request): RedirectResponse
{
$url = $request->query->get(self::RETURN_URL_PARAM);
if (is_string($url) && $this->isSafeUrl($url, $request->getBaseUrl())) {
return new RedirectResponse($url);
}

throw $this->createAccessDeniedException('Malformed return URL');
}

private function isSafeUrl(string $referer, string $baseUrl): bool
{
return str_starts_with($referer, $baseUrl);
}
}
8 changes: 8 additions & 0 deletions src/bundle/Resources/config/routing.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -951,3 +951,11 @@ ibexa.permission.limitation.language.content_read:
methods: [GET]
requirements:
contentInfoId: \d+


### User Mode

ibexa.user_mode.change:
path: /user/change-mode
controller: 'Ibexa\Bundle\AdminUi\Controller\User\UserModeController::changeAction'
methods: [GET, POST]
6 changes: 6 additions & 0 deletions src/bundle/Resources/config/services/controllers.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,12 @@ services:
tags:
- controller.service_arguments

Ibexa\Bundle\AdminUi\Controller\User\UserModeController:
parent: Ibexa\Contracts\AdminUi\Controller\Controller
autowire: true
tags:
- controller.service_arguments

Ibexa\Bundle\AdminUi\Controller\UserOnTheFlyController:
parent: Ibexa\Contracts\AdminUi\Controller\Controller
autowire: true
Expand Down
8 changes: 8 additions & 0 deletions src/bundle/Resources/config/services/twig.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,11 @@ services:
- '@Ibexa\AdminUi\Limitation\Templating\LimitationBlockRenderer'
tags:
- { name: twig.extension }

Ibexa\Bundle\AdminUi\Templating\Twig\UserModeExtension:
tags:
- { name: twig.extension }

Ibexa\Bundle\AdminUi\Templating\Twig\LocationExtension:
tags:
- { name: twig.extension }
1 change: 1 addition & 0 deletions src/bundle/Resources/encore/ibexa.js.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ const layout = [
path.resolve(__dirname, '../public/js/scripts/admin.anchor.navigation'),
path.resolve(__dirname, '../public/js/scripts/admin.context.menu'),
path.resolve(__dirname, '../public/js/scripts/admin.focus.mode.js'),
path.resolve(__dirname, '../public/js/scripts/admin.user.mode.js'),
path.resolve(__dirname, '../public/js/scripts/sidebar/main.menu.js'),
path.resolve(__dirname, '../public/js/scripts/admin.input.text.js'),
path.resolve(__dirname, '../public/js/scripts/admin.table.js'),
Expand Down
5 changes: 2 additions & 3 deletions src/bundle/Resources/public/js/scripts/admin.location.view.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
const mfuContainer = doc.querySelector('#ibexa-mfu');
const token = doc.querySelector('meta[name="CSRF-Token"]').content;
const siteaccess = doc.querySelector('meta[name="SiteAccess"]').content;
const sortContainer = doc.querySelector('[data-sort-field][data-sort-order]');
const emdedItemsUpdateChannel = new BroadcastChannel('ibexa-emded-item-live-update');
const queryString = window.location.search;
const urlParams = new URLSearchParams(queryString);
Expand Down Expand Up @@ -126,8 +125,8 @@
};

listContainers.forEach((container) => {
const sortField = sortContainer.getAttribute('data-sort-field');
const sortOrder = sortContainer.getAttribute('data-sort-order');
const sortField = container.getAttribute('data-sort-field');
const sortOrder = container.getAttribute('data-sort-order');
const subitemsRoot = ReactDOM.createRoot(container);
const parentLocationId = parseInt(container.dataset.location, 10);
const activeView = getLocationActiveView(parentLocationId);
Expand Down
12 changes: 12 additions & 0 deletions src/bundle/Resources/public/js/scripts/admin.user.mode.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
(function (global, doc) {
const FORM_SELECTOR = 'form[name=user_mode_change]';
const form = doc.querySelector(FORM_SELECTOR);

if (form) {
form.querySelectorAll('input[type=checkbox]').forEach((input) => {
input.addEventListener('change', () => {
form.submit();
});
});
}
})(window, window.document);
10 changes: 10 additions & 0 deletions src/bundle/Resources/translations/ibexa_admin_ui.en.xliff
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,16 @@
<target state="new">Removed '%languageCode%' translation from '%name%'.</target>
<note>key: translation.remove.success</note>
</trans-unit>
<trans-unit id="0f313126239afeaa231f0b3d612157cf45a2bc18" resname="user.mode.expert">
<source>Expert mode</source>
<target state="new">Expert mode</target>
<note>key: user.mode.expert</note>
</trans-unit>
<trans-unit id="51322eb3bc0a8251318410f06eb9d1595727d749" resname="user.mode.smart">
<source>Smart mode</source>
<target state="new">Smart mode</target>
<note>key: user.mode.smart</note>
</trans-unit>
<trans-unit id="885ec7aab950e05f729031321cddbb6d7f9c3246" resname="version.delete.success">
<source>Removed version(s) from '%name%'.</source>
<target state="new">Removed version(s) from '%name%'.</target>
Expand Down
20 changes: 15 additions & 5 deletions src/bundle/Resources/translations/ibexa_locationview.en.xliff
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,16 @@
<target state="new">Location ID</target>
<note>key: tab.author.location_id</note>
</trans-unit>
<trans-unit id="d3e1f764a54cdc6e1a8e0c1d197c9d439f734b31" resname="tab.author.modified">
<source>Modified</source>
<target state="new">Modified</target>
<note>key: tab.author.modified</note>
</trans-unit>
<trans-unit id="49a9415aa25635c21a41acd68ca891c7320923c0" resname="tab.author.published">
<source>Published</source>
<target state="new">Published</target>
<note>key: tab.author.published</note>
</trans-unit>
<trans-unit id="4fd9a40170cf4bc5acff719c14eaf2d15fc9942d" resname="tab.details.change_section ">
<source>Change section</source>
<target state="new">Change section</target>
Expand Down Expand Up @@ -266,6 +276,11 @@
<target state="new">Authors</target>
<note>key: tab.name.authors</note>
</trans-unit>
<trans-unit id="cbbcbdffdfb49411175c7039130d7ae05a5af6cf" resname="tab.name.data">
<source>Data</source>
<target state="new">Data</target>
<note>key: tab.name.data</note>
</trans-unit>
<trans-unit id="5d92b84a6c78bd443010f172c47d01ce0dacacd7" resname="tab.name.details">
<source>Technical Details</source>
<target state="new">Technical Details</target>
Expand All @@ -281,11 +296,6 @@
<target state="new">Policies</target>
<note>key: tab.name.policies</note>
</trans-unit>
<trans-unit id="eeca4145269fdae93e31aba7725f260e0c44839a" resname="tab.name.quick_preview">
<source>Quick Preview</source>
<target state="new">Quick Preview</target>
<note>key: tab.name.quick_preview</note>
</trans-unit>
<trans-unit id="8954433abc5d011c00f81f317c7eebec41fbb1c0" resname="tab.name.relations">
<source>Relations</source>
<target state="new">Relations</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,21 @@
label: 'tab.author.creator'|trans()|desc('Creator'),
content: creator_name,
},
{
label: 'tab.author.published'|trans|desc('Published'),
content: content_info.publishedDate | ibexa_full_datetime
},
{
is_break: true
},
{
label: 'tab.author.last_contributor'|trans()|desc('Last contributor'),
content: last_contributor_name,
}
},
{
label: 'tab.author.modified'|trans|desc('Modified'),
content: content_info.modificationDate | ibexa_full_datetime
},
] %}

{% include '@ibexadesign/ui/component/details/details.html.twig' with {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,20 +131,20 @@
{% include '@ibexadesign/ui/component/table/table_header.html.twig' with {
headline: 'tab.details.sub_items_sorting_order'|trans|desc('Sub-item sorting order'),
} only %}
<div class="bg-white ml-4" data-sort-field="{{ sort_field_clause_map[location.sortField] }}" data-sort-order="{{ location.sortOrder ? 'ascending' : 'descending' }}">

<div class="bg-white ml-4">
{{ form_start(form_location_update, {
'method': 'POST',
'action': path('ibexa.location.update'),
'attr': {'class': 'form-inline ibexa-form-inline justify-content-start'}
}) }}

{{ form_label(form_location_update.sort_field, 'tab.details.sub_items_listing_by.order_by'|trans|desc('Order by')) }}
{{ form_widget(form_location_update.sort_field, { 'attr': {'class': 'ibexa-form-autosubmit ml-2'} }) }}

{{ form_label(form_location_update.sort_order, 'tab.details.sub_items_listing_by.in'|trans|desc('in')) }}
{{ form_widget(form_location_update.sort_order, { 'attr': {'class': 'ibexa-form-autosubmit ml-2'} }) }}

{% do form_location_update.update.setRendered() %}
{{ form_end(form_location_update) }}
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,14 @@

{% set body_row_cols = body_row_cols|merge([
{ content: translation.name },
{ content: translation.languageCode },
]) %}

{% if ibexa_is_expert_mode() %}
Copy link
Contributor

Choose a reason for hiding this comment

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

I recommend utilizing ibexa_user_settings['user_mode'] instead of a dedicated Twig function.

If we name the function with the "expert" keyword, this designation will remain with us for the upcoming years. If we decide to change the name or introduce additional user modes, we will need to implement extra logic

Copy link
Contributor

Choose a reason for hiding this comment

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

Or ibexa_mode(...) function if we cannot rely on user settings being present in the template.

Copy link
Contributor

Choose a reason for hiding this comment

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

Or even, we can use wildcards in twig function names and do it like ibexa_is_*_mode and handle it that way.

Copy link
Contributor

@Steveb-p Steveb-p Dec 1, 2023

Choose a reason for hiding this comment

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

One issue with wildcards is that you cannot use mode value as a variable.

You have to use the full function name immediately.

{% set expected_mode = 'expert' %}
{{ expected_mode is same as 'expert' ? ibexa_is_expert_mode() : ibexa_is_smart_mode() }}

Using an argument does not have this limitation.

Copy link
Member Author

Choose a reason for hiding this comment

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

ibexa_user_settings['user_mode'] or ibexa_mode(...) is a no go for me. It's simply bad DX:

{% if ibexa_user_settings['user_mode'] === constant('\\Ibexa\\AdminUi\\UserSetting\\UserMode::SMART') %}
    Do something
{% endif %}

Wildcard function is a good idea, but we don't need it at the moment.

{% set body_row_cols = body_row_cols|merge([
{ content: translation.languageCode }
]) %}
{% endif %}

{% if main_translation_switch %}
{% set col_raw %}
<input
Expand All @@ -55,8 +60,14 @@
{% set head_cols = [
{ has_checkbox: true },
{ content: 'tab.translations.language_name'|trans|desc('Language name') },
{ content: 'tab.translations.language_code'|trans|desc('Language code') },
] %}

{% if ibexa_is_expert_mode() %}
{% set head_cols = head_cols|merge([
{ content: 'tab.translations.language_code'|trans|desc('Language code') }
]) %}
{% endif %}

{% if main_translation_switch %}
{% set head_cols = head_cols|merge([{
content: 'tab.translations.main_language'|trans|desc('Main language'),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,6 @@
data-content-types="{{ subitems_module.content_type_info_list }}"
data-udw-config-bulk-move-items="{{ udwConfigSingle }}"
data-udw-config-bulk-add-location="{{ udwConfigSingle }}"
data-sort-field="{{ ibexa_location_sort_field_as_rest_sort_clause(location) }}"
data-sort-order="{{ ibexa_location_sort_order_as_rest_value(location) }}"
></div>
13 changes: 11 additions & 2 deletions src/bundle/Resources/views/themes/admin/ui/form_fields.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,9 @@
{%- endfor -%}
{%- endblock -%}

{%- block search_widget -%}
{%- block search_widget -%}
{% set has_search = true %}

{{ block('form_widget_simple') }}
{%- endblock -%}

Expand Down Expand Up @@ -554,3 +554,12 @@
</div>
</div>
{%- endblock -%}

{% block user_mode_toggle_widget %}
{% set label_on = 'user.mode.expert'|trans({}, 'ibexa_admin_ui')|desc('Expert mode') %}
{% set label_off = 'user.mode.smart'|trans({}, 'ibexa_admin_ui')|desc('Smart mode') %}
{% set small = true %}
{% set checked = ibexa_is_expert_mode() %}

{{ block('toggle_widget') }}
{% endblock %}
Loading
Loading