IBX-6398: UDW as standalone as GH package #1010

lucasOsti · 2023-11-24T14:45:28Z

Question	Answer
Tickets	https://issues.ibexa.co/browse/IBX-6967
Bug fix?	yes/no
New feature?	yes/no
BC breaks?	yes/no
Tests pass?	yes/no
Doc needed?	yes/no
License	GPL-2.0

Related PR's:

Checklist:

Coding standards ($ composer fix-cs)
Ready for Code Review

GrabowskiM · 2023-12-07T12:43:30Z

src/bundle/Resources/public/js/scripts/helpers/icon.helper.js

+    }
+
+    const iconSetPath = adminUiConfig.iconPaths.iconSets[iconSet];
+    return `${iconSetPath}#${path}`;


Suggested change

return `${iconSetPath}#${path}`;

return `${iconSetPath}#${path}`;

GrabowskiM · 2023-12-07T12:45:28Z

src/bundle/Resources/public/js/scripts/helpers/location.helper.js

+import { getJsonFromResponse } from './request.helper';
+import { showErrorNotification } from './notification.helper';
+import { getRestInfo, getTranslator } from './context.helper';
+import { getRequestHeaders, getRequestMode } from '../../../../../ui-dev/src/modules/common/services/common.service';


what would you say on using alias here as well? @ibexa-admin-ui/src/bundle/ui-dev/src/modules/common/services/common.service?
That's something we could do in future in other places as well, to avoid that many ../ (and remove thinking "is it enough ../ or I need one more? and moving files between directories)
@dew326 @Gengar-i @tischsoic @lucasOsti

Current upside of using ../../.. is that VS Code understands where a file is. If you use an alias, this file won't be recognized

GrabowskiM · 2023-12-07T12:51:37Z

src/bundle/ui-dev/src/modules/content-tree/content.tree.module.js

@@ -355,6 +356,7 @@ export default class ContentTreeModule extends Component {

    render() {
        const { onClickItem, subitemsLimit, subitemsLoadLimit, treeMaxDepth, userId, resizable } = this.props;
+


why new line here?

GrabowskiM · 2023-12-07T12:52:23Z

src/bundle/ui-dev/src/modules/universal-discovery/bookmarks.tab.module.js

@@ -1,5 +1,7 @@
 import React, { useContext, useState, useEffect, useRef } from 'react';

+import { getIconPath } from '@ibexa-admin-ui/src/bundle/Resources/public/js/scripts/helpers/icon.helper';


maybe move it to line 21/22, to other helpers? :)

GrabowskiM · 2023-12-07T12:52:34Z

src/bundle/ui-dev/src/modules/universal-discovery/browse.tab.module.js

@@ -1,13 +1,14 @@
 import React, { useContext } from 'react';

+import { getIconPath } from '@ibexa-admin-ui/src/bundle/Resources/public/js/scripts/helpers/icon.helper';


same as above

GrabowskiM · 2023-12-07T12:57:16Z

src/bundle/ui-dev/src/modules/universal-discovery/content.meta.preview.module.js

@@ -1,10 +1,14 @@
 import React, { useContext, useEffect, useMemo, useRef } from 'react';

+import { formatShortDateTime } from '@ibexa-admin-ui/src/bundle/Resources/public/js/scripts/helpers/timezone.helper';


to other helpers, they're so sad being separated 😢

GrabowskiM · 2023-12-07T12:57:38Z

src/bundle/ui-dev/src/modules/universal-discovery/search.tab.module.js

@@ -1,11 +1,12 @@
 import React, { useContext, useEffect } from 'react';

+import { getIconPath } from '@ibexa-admin-ui/src/bundle/Resources/public/js/scripts/helpers/icon.helper';


as above (I think)

GrabowskiM · 2023-12-07T12:59:16Z

src/bundle/ui-dev/src/modules/universal-discovery/universal.discovery.module.js

+    parse as parseTooltips,
+    hideAll as hideAllTooltips,
+} from '@ibexa-admin-ui/src/bundle/Resources/public/js/scripts/helpers/tooltips.helper';
+import { getAdminUiConfig, getTranslator } from '@ibexa-admin-ui/src/bundle/Resources/public/js/scripts/helpers/context.helper';


These helpers are happy, they're together till the end of time! 🥳

Sorry, to much code, my brain explodes...

GrabowskiM · 2023-12-07T12:59:35Z

src/bundle/ui-dev/src/modules/universal-discovery/universal.discovery.module.js

+        selectedLabel: () => {
+            return (
+                <div className="c-simple-dropdown__option-label">
+                    {getTranslator.trans(/*@Desc("Sort by date")*/ 'sorting.date.selected_label', {}, 'ibexa_universal_discovery_widget')}


Suggested change

{getTranslator.trans(/*@Desc("Sort by date")*/ 'sorting.date.selected_label', {}, 'ibexa_universal_discovery_widget')}

{getTranslator().trans(/*@Desc("Sort by date")*/ 'sorting.date.selected_label', {}, 'ibexa_universal_discovery_widget')}

GrabowskiM · 2023-12-07T12:59:46Z

src/bundle/ui-dev/src/modules/universal-discovery/universal.discovery.module.js

+        selectedLabel: () => {
+            return (
+                <div className="c-simple-dropdown__option-label">
+                    {getTranslator.trans(/*@Desc("Sort by date")*/ 'sorting.date.selected_label', {}, 'ibexa_universal_discovery_widget')}


Suggested change

{getTranslator.trans(/*@Desc("Sort by date")*/ 'sorting.date.selected_label', {}, 'ibexa_universal_discovery_widget')}

{getTranslator().trans(/*@Desc("Sort by date")*/ 'sorting.date.selected_label', {}, 'ibexa_universal_discovery_widget')}

Gengar-i · 2023-12-07T13:20:58Z

src/bundle/Resources/config/services/controllers.yaml

@@ -222,5 +222,10 @@ services:
    Ibexa\Bundle\AdminUi\Controller\Permission\LanguageLimitationController:
        parent: Ibexa\Contracts\AdminUi\Controller\Controller
        autowire: true
+
+


Suggested change

Gengar-i · 2023-12-07T15:10:20Z

src/bundle/Resources/public/js/scripts/helpers/system.helper.js

+    isWindows,
+    isMac,
+    isLinux,


Suggested change

isWindows,

isMac,

isLinux,

get isWindows() { return isWindows(); }

@lucasOsti, @GrabowskiM, @tischsoic, @dew326
What do you think about this approach?
I can add it in seperate PR.
So we can get property like helper.isWindows instead of helper.isWindows() when we only return bool true or false

I don't think that avoiding () is enough to replace function with getter. What if e.g. in the future we would like to pass a parameter?

Gengar-i · 2023-12-07T15:25:14Z

src/bundle/Resources/public/js/scripts/helpers/tooltips.helper.js

+        hide: delayHide ? parseInt(delayHide, 10) : 75,
+    };
+    const extraClass = tooltipNode.dataset.tooltipExtraClass ?? '';
+    const placement = tooltipNode.dataset.tooltipPlacement ?? 'bottom';


Suggested change

const placement = tooltipNode.dataset.tooltipPlacement ?? 'bottom';

const placement = tooltipNode.dataset.bsPlacement ?? 'bottom';

There suppose to be other property. I checked bootstrap docks they are using:
data-placement="top"
and in BS 5.0:
data-bs-placement="top"

Gengar-i · 2023-12-07T15:28:52Z

src/bundle/ui-dev/src/modules/common/pagination/pagination.info.js

-const { Translator } = window;
-
-const PaginationInfo = ({ totalCount, viewingCount, extraClasses }) => {
+const PaginationInfo = ({ totalCount, viewingCount, extraClasses, Translator }) => {


const Translator = getTranslator(); maybe?

Gengar-i · 2023-12-07T15:36:48Z

src/bundle/ui-dev/src/modules/universal-discovery/content.create.tab.module.js

    const iframeRef = createRef();
+    const iframeUrl = () => {


Suggested change

const iframeUrl = () => {

const iframe = {

get url() {

const { locationId, languageCode, contentTypeIdentifier } = contentOnTheFlyData;

return Routing.generate('ibexa.content.on_the_fly.create', {

locationId,

languageCode,

contentTypeIdentifier,

});

}

};

// Use

console.log(iframe.url);

@GrabowskiM will be happy :>
We can also move ref here

tischsoic · 2023-12-07T19:31:04Z

src/bundle/Resources/public/js/scripts/helpers/context.helper.js

+export const getAdminUiConfig = () => adminUiConfig;
+export const getBootstrap = () => bootstrap;
+export const getFlatpickr = () => flatpickr;
+export const getMoment = () => moment;
+export const getPopper = () => Popper;
+export const getRouting = () => Routing;
+export const getTranslator = () => Translator;
+export const getRestInfo = () => restInfo;


Maybe we should have one function getContext and we would have:

import { getContext } from './context.helper'; const { Translator, moment } = getContext();

tischsoic · 2023-12-07T19:31:49Z

src/bundle/Resources/public/js/scripts/helpers/config.loader.js

+import * as tooltips from './tooltips.helper';
+import * as user from './user.helper';
+
+(function (ibexa) {


Do we need IIFE? I think given we have modules this is not needed.

tischsoic · 2023-12-07T19:32:31Z

src/bundle/Resources/public/js/scripts/helpers/config.loader.js

+    ibexa.addConfig('helpers.contentType', contentType);
+    ibexa.addConfig('helpers.cookies', cookies);
+    ibexa.addConfig('helpers.formError', formError);


I don't understand, why we have config.loader.js?

Because I create config.loader.js file wherever something was added to window.ibexa via the addConfig method

tischsoic · 2023-12-11T09:00:05Z

src/bundle/Resources/public/js/scripts/helpers/context.helper.js

+    }
+};
+
+export const getAdminUiConfig = () => adminUiConfig;


As I proposed on Slack, we can avoid such functions by adding export default on object with getters and setters. (could be done in a followup PR). Example:

… (#2113)

sonarcloud · 2023-12-13T14:20:58Z

Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

3 New issues
0 Security Hotspots
No data about Coverage
2.2% Duplication on New Code

See analysis details on SonarCloud

github-actions bot assigned ciastektk, mikadamczyk, Nattfarinn, ViniTou and dew326 Nov 24, 2023

adamwojs changed the title ~~Ibx 6398 udw as gh package~~ IBX-06398: UDW as standalone as GH package Nov 25, 2023

adamwojs changed the title ~~IBX-06398: UDW as standalone as GH package~~ IBX-6398: UDW as standalone as GH package Nov 25, 2023

lucasOsti force-pushed the IBX-6398-UDW-as-GH-package branch from 300f85d to da3193a Compare November 27, 2023 15:53

lucasOsti force-pushed the IBX-6398-UDW-as-GH-package branch 2 times, most recently from 86cdf23 to 10eb46f Compare December 7, 2023 09:54

GrabowskiM requested changes Dec 7, 2023

View reviewed changes

lucasOsti requested review from tischsoic, dew326 and Gengar-i December 7, 2023 13:30

Gengar-i approved these changes Dec 7, 2023

View reviewed changes

lucasOsti force-pushed the IBX-6398-UDW-as-GH-package branch 2 times, most recently from 1fedbe0 to 216b298 Compare December 7, 2023 19:33

lucasOsti requested review from GrabowskiM and Gengar-i December 7, 2023 19:36

tischsoic reviewed Dec 7, 2023

View reviewed changes

Gengar-i approved these changes Dec 8, 2023

View reviewed changes

dew326 approved these changes Dec 8, 2023

View reviewed changes

tischsoic reviewed Dec 11, 2023

View reviewed changes

tischsoic approved these changes Dec 11, 2023

View reviewed changes

micszo self-assigned this Dec 12, 2023

micszo mentioned this pull request Dec 12, 2023

Run regression for IBX-6398 ibexa/commerce#525

Closed

lucasOsti force-pushed the IBX-6398-UDW-as-GH-package branch from a58178c to 3c3af15 Compare December 12, 2023 09:49

This was referenced Dec 12, 2023

Run regression for IBX-6398 ibexa/experience#295

Closed

Run regression for IBX-6398 ibexa/headless#41

Closed

Run regression for IBX-6398 ibexa/oss#128

Closed

lucasOsti and others added 19 commits December 13, 2023 12:28

IBX-6398: UDW as GH package

be0d6b4

Prettier

5d88471

IBX-6398: UDW as GH package

8a996f9

UDW as GH package

de1b307

IBX-6398: UDW as GH package

810a364

UDW as GH package

9fc2db6

UDW as GH package

af9dd96

UDW as GH package

15361d5

Added ApplicationConfigRestResolver

6c6b840

[PHPStan] Regenerated baseline

6b995af

UDW as GH package

3039dcf

UDW as GH package

87dcd58

UDW as GH package

c6ed09e

UDW as GH package

aced7a8

UDW as GH package

6dba3df

UDW as GH package

81d8680

UDW as GH package

fc4dca9

UDW as GH package

afdd7ed

UDW as GH package

d46625f

lucasOsti force-pushed the IBX-6398-UDW-as-GH-package branch from 3c3af15 to d46625f Compare December 13, 2023 11:33

lucasOsti and others added 5 commits December 13, 2023 14:03

UDW as GH package

72a74ad

After CR

a86da4b

UDW as GH package

1b7a7c9

Fixed fulltextCriterion

0a70372

IBX-1464: Added versionNo param in the ezimage edit form template…

e4aba74

… (#2113)

micszo approved these changes Dec 14, 2023

View reviewed changes

micszo removed their assignment Dec 14, 2023

lucasOsti merged commit 15704ef into main Dec 14, 2023
16 of 19 checks passed

webhdx deleted the IBX-6398-UDW-as-GH-package branch December 14, 2023 10:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IBX-6398: UDW as standalone as GH package #1010

IBX-6398: UDW as standalone as GH package #1010

lucasOsti commented Nov 24, 2023 •

edited

Loading

GrabowskiM Dec 7, 2023

GrabowskiM Dec 7, 2023

tischsoic Dec 7, 2023

GrabowskiM Dec 7, 2023

GrabowskiM Dec 7, 2023

GrabowskiM Dec 7, 2023

GrabowskiM Dec 7, 2023

GrabowskiM Dec 7, 2023

GrabowskiM Dec 7, 2023

GrabowskiM Dec 7, 2023

GrabowskiM Dec 7, 2023

Gengar-i Dec 7, 2023

Gengar-i Dec 7, 2023

tischsoic Dec 7, 2023 •

edited

Loading

Gengar-i Dec 7, 2023

Gengar-i Dec 7, 2023

Gengar-i Dec 7, 2023

tischsoic Dec 7, 2023

tischsoic Dec 7, 2023

tischsoic Dec 7, 2023

lucasOsti Dec 7, 2023

tischsoic Dec 11, 2023

sonarcloud bot commented Dec 13, 2023

	return `${iconSetPath}#${path}`;

	return `${iconSetPath}#${path}`;

		@@ -355,6 +356,7 @@ export default class ContentTreeModule extends Component {

		render() {
		const { onClickItem, subitemsLimit, subitemsLoadLimit, treeMaxDepth, userId, resizable } = this.props;

		@@ -1,5 +1,7 @@
		import React, { useContext, useState, useEffect, useRef } from 'react';

		import { getIconPath } from '@ibexa-admin-ui/src/bundle/Resources/public/js/scripts/helpers/icon.helper';

		@@ -1,13 +1,14 @@
		import React, { useContext } from 'react';

		import { getIconPath } from '@ibexa-admin-ui/src/bundle/Resources/public/js/scripts/helpers/icon.helper';

		@@ -1,10 +1,14 @@
		import React, { useContext, useEffect, useMemo, useRef } from 'react';

		import { formatShortDateTime } from '@ibexa-admin-ui/src/bundle/Resources/public/js/scripts/helpers/timezone.helper';

		@@ -1,11 +1,12 @@
		import React, { useContext, useEffect } from 'react';

		import { getIconPath } from '@ibexa-admin-ui/src/bundle/Resources/public/js/scripts/helpers/icon.helper';

	{getTranslator.trans(/@Desc("Sort by date")/ 'sorting.date.selected_label', {}, 'ibexa_universal_discovery_widget')}
	{getTranslator().trans(/@Desc("Sort by date")/ 'sorting.date.selected_label', {}, 'ibexa_universal_discovery_widget')}

	const placement = tooltipNode.dataset.tooltipPlacement ?? 'bottom';
	const placement = tooltipNode.dataset.bsPlacement ?? 'bottom';

-    const iframeUrl = () => {
+   const iframe = {
+    get url() {
+        const { locationId, languageCode, contentTypeIdentifier } = contentOnTheFlyData;
+        return Routing.generate('ibexa.content.on_the_fly.create', {
+            locationId,
+            languageCode,
+            contentTypeIdentifier,
+        });
+    }
+};
+// Use
+console.log(iframe.url);

IBX-6398: UDW as standalone as GH package #1010

IBX-6398: UDW as standalone as GH package #1010

Conversation

lucasOsti commented Nov 24, 2023 • edited Loading

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tischsoic Dec 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Dec 13, 2023

Quality Gate passed

lucasOsti commented Nov 24, 2023 •

edited

Loading

tischsoic Dec 7, 2023 •

edited

Loading