EZP-31345 Added language select when user edit sub items #270

lucasOsti · 2020-02-17T12:16:09Z

Question	Answer
Tickets	https://jira.ez.no/browse/EZP-31345
Bug fix?	no
New feature?	yes
BC breaks?	no
Tests pass?	yes
Doc needed?	no
License	GPL-2.0

Checklist:

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

GrabowskiM · 2020-02-18T08:57:20Z

src/modules/sub-items/components/sub-items-list/instant.filter.component.js

+
+    useEffect(() => {
+        const items = [..._refInstantFilter.current.querySelectorAll(`.${props.itemClass}`)];
+        const itemsMap = items.reduce(


Why reduce instead of simply map?

GrabowskiM · 2020-02-18T11:13:59Z

src/modules/sub-items/components/sub-items-list/instant.filter.component.js

+    }, []);
+
+    useEffect(() => {
+        window.clearTimeout(filterTimeout);


clearTimeout should go to return function insteaed of here

Suggested change

window.clearTimeout(filterTimeout);

useEffect(() => {

const filterActionTimeout = window.setTimeout(() => {

const results = itemsMap.filter((item) => item.label.includes(filterQuery.toLowerCase()));

itemsMap.forEach((item) => item.element.setAttribute('hidden', true));

results.forEach((item) => item.element.removeAttribute('hidden'));

}, FILTER_TIMEOUT);

return () => {

window.clearTimeout(filterActionTimeout);

}

}, [filterQuery]);

GrabowskiM · 2020-02-18T11:15:57Z

src/modules/sub-items/components/sub-items-list/instant.filter.component.js

+        window.clearTimeout(filterTimeout);
+
+        const filterActionTimeout = window.setTimeout(() => {
+            const results = itemsMap.filter((item) => item.label.includes(filterQuery.toLowerCase()));


You can store filterQuery.toLowerCase() in new variable so that you won't fire toLowerCase with each filter iteration

GrabowskiM · 2020-02-18T11:17:05Z

src/modules/sub-items/components/sub-items-list/instant.filter.component.js

+                                    name="items"
+                                    className="form-check-input"
+                                    value={item.value}
+                                    onChange={(event) => props.handleItemChange(event)}


Could be just onChange={props.handleItemChange}

GrabowskiM · 2020-02-18T11:19:45Z

src/modules/sub-items/components/table-view/table.view.item.component.js

+        const languages = this.props.languages.mappings;
+        const { languageCodes } = this.props.item.content._info.currentVersion;
+        const label = Translator.trans(/*@Desc("Select language")*/ 'languages.modal.label', {}, 'sub_items');
+        const languageItems = languageCodes.reduce(


Why reduce instead of simply map?

Suggested change

const languageItems = languageCodes.reduce(

const languageItems = languageCodes.map((item) => ({

label: languages[item].name,

value: item,

}));

GrabowskiM · 2020-02-18T11:20:37Z

src/modules/sub-items/components/table-view/table.view.item.component.js

+                    </div>
+                </div>
+            );
+        } else {


Unnecessary else

Suggested change

} else {

}

return false;

}

}

GrabowskiM · 2020-02-18T11:21:39Z

src/modules/sub-items/sub.items.module.js

+     * @memberof SubItemsModule
+     */
+    setContentEditLanguagesModalPosition() {
+        const rightMainMenuNode = document.querySelector(RIGHT_MAIN_MENU_SELECTOR);


Could go inside if

dew326

Review discussed in private, some changes in the architecture are needed

dew326 · 2020-02-20T07:12:20Z

Resources/public/scss/modules/sub-items-list/_language.selector.scss

+        }
+    }
+}
+


Suggested change

dew326 · 2020-02-20T08:00:35Z

src/modules/sub-items/components/sub-items-list/instant.filter.component.js

+    const _refInstantFilter = useRef(null);
+    const [filterQuery, setFilterQuery] = useState('');
+    const [itemsMap, setItemsMap] = useState([]);
+


Suggested change

src/modules/sub-items/components/sub-items-list/instant.filter.component.js

dew326 · 2020-02-20T08:02:04Z

src/modules/sub-items/components/sub-items-list/instant.filter.component.js

+};
+
+InstantFilter.propTypes = {
+    uniqueId: PropTypes.string,


I think this is not needed anymore. I current implementation there will be only one InstantFilter and it will always be the same value.

dew326 · 2020-02-20T08:04:00Z

src/modules/sub-items/components/sub-items-list/instant.filter.component.js

+InstantFilter.propTypes = {
+    uniqueId: PropTypes.string,
+    items: PropTypes.array,
+    itemClass: PropTypes.string,


Is this have to be in prop? It can be just regular variable. I don't see a point of overriding this by parent component, or am I missing something.

dew326 · 2020-02-20T08:04:19Z

src/modules/sub-items/components/sub-items-list/language.selector.compoment.js

+const LanguageSelector = (props) => {
+    const className = createCssClassNames({
+        'ez-extra-actions': true,
+        'ez-language-selector': true,


Suggested change

'ez-language-selector': true,

'c-language-selector': true,

dew326 · 2020-02-20T08:08:16Z

src/modules/sub-items/components/table-view/table.view.component.js

@@ -105,6 +113,31 @@ export default class TableViewComponent extends Component {
        );
    }

+    /**
+     * Set language selector data


Suggested change

* Set language selector data

* Sets language selector data

dew326 · 2020-02-20T14:38:14Z

src/modules/sub-items/components/sub-items-list/instant.filter.component.js

+                <input
+                    type="text"
+                    className="ez-instant-filter__input form-control"
+                    placeholder="Type to refine"


I think we need translation

bogusez · 2020-02-21T12:39:08Z

I've got a problem with editing sub-items after cloning this. You can find more info on attached screenshot below.

lucasOsti added the Ready for review label Feb 17, 2020

lucasOsti requested review from GrabowskiM and dew326 February 17, 2020 12:16

GrabowskiM requested changes Feb 18, 2020

View reviewed changes

lucasOsti requested a review from GrabowskiM February 18, 2020 15:05

dew326 requested changes Feb 19, 2020

View reviewed changes

lucasOsti requested a review from dew326 February 19, 2020 14:32

GrabowskiM approved these changes Feb 19, 2020

View reviewed changes

dew326 requested changes Feb 20, 2020

View reviewed changes

lucasOsti requested a review from dew326 February 20, 2020 09:16

dew326 reviewed Feb 20, 2020

View reviewed changes

lucasOsti requested a review from dew326 February 20, 2020 14:58

dew326 approved these changes Feb 20, 2020

View reviewed changes

lucasOsti added Ready for QA and removed Ready for review labels Feb 20, 2020

lucasOsti requested a review from dew326 February 21, 2020 08:12

lucasOsti removed the Ready for QA label Feb 21, 2020

dew326 approved these changes Feb 21, 2020

View reviewed changes

lucasOsti added the Ready for QA label Feb 21, 2020

lucasOsti added 8 commits February 21, 2020 14:01

Added edit sub items from list

1fa686c

Select language when editing sub items

1e5c28e

EZP-31345 Added language select when user edit sub items

a3a5c64

After code review

0e156ad

after code review

30b6a02

After code review

feb22bb

After code review

e050719

Added translation for instant filter placeholder

31ff69d

lucasOsti force-pushed the EZP-31345-edit-subitems-form-list branch from 8d56eea to 31ff69d Compare February 21, 2020 13:11

bogusez approved these changes Feb 24, 2020

View reviewed changes

bogusez added QA approved and removed Ready for QA labels Feb 24, 2020

lserwatka merged commit 4dda573 into ezsystems:master Feb 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EZP-31345 Added language select when user edit sub items #270

EZP-31345 Added language select when user edit sub items #270

lucasOsti commented Feb 17, 2020

GrabowskiM Feb 18, 2020

GrabowskiM Feb 18, 2020

GrabowskiM Feb 18, 2020

GrabowskiM Feb 18, 2020

GrabowskiM Feb 18, 2020

GrabowskiM Feb 18, 2020

GrabowskiM Feb 18, 2020

GrabowskiM Feb 18, 2020

dew326 left a comment

dew326 Feb 20, 2020

dew326 Feb 20, 2020

dew326 Feb 20, 2020

dew326 Feb 20, 2020

dew326 Feb 20, 2020

dew326 Feb 20, 2020

dew326 Feb 20, 2020

bogusez commented Feb 21, 2020 •

edited

Loading

-        window.clearTimeout(filterTimeout);
+      useEffect(() => {
+        const filterActionTimeout = window.setTimeout(() => {
+            const results = itemsMap.filter((item) => item.label.includes(filterQuery.toLowerCase()));
+            itemsMap.forEach((item) => item.element.setAttribute('hidden', true));
+            results.forEach((item) => item.element.removeAttribute('hidden'));
+        }, FILTER_TIMEOUT);
+        return () => {
+          window.clearTimeout(filterActionTimeout);
+        }
+    }, [filterQuery]);

-        const languageItems = languageCodes.reduce(
+        const languageItems = languageCodes.map((item) => ({
+                    label: languages[item].name,
+                    value: item,
+        }));

EZP-31345 Added language select when user edit sub items #270

EZP-31345 Added language select when user edit sub items #270

Conversation

lucasOsti commented Feb 17, 2020

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

dew326 left a 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

bogusez commented Feb 21, 2020 • edited Loading

bogusez commented Feb 21, 2020 •

edited

Loading