EZP-29182/EZP-29187: Improvements in Universal Discovery Widget single and multi selection #94

dew326 · 2018-07-18T12:13:07Z

Jira tickets: https://jira.ez.no/browse/EZP-29187, https://jira.ez.no/browse/EZP-29182

Screenshots

…e and multi selection

sunpietro · 2018-07-18T12:24:45Z

src/modules/universal-discovery/components/content-table/content.table.item.component.js

-    const contentType = contentTypesMap ? contentTypesMap[item.ContentType._href] : false;
-    const contentTypeName = contentType ? contentType.names.value[0]['#text'] : labels.contentTableItem.notAvailable;
-    const onClick = !!onItemClick ? onItemClick.bind(null, data) : null;
+export default class ContentTableItemComponent extends Component {


You should go back to stateless component. The renderSelectContentBtn can be defined outside of const ContentTableItemComponent definition, just pass the props to it.

Good catch. During development I changed this to component with state and then removed the state :)

sunpietro · 2018-07-18T12:26:38Z

src/modules/universal-discovery/components/finder/finder.tree.branch.component.js

-            <button
-                className="c-finder-tree-branch__load-more"
-                onClick={() => this.props.onLoadMore(this.props.parentLocation)}>
+            <button className="c-finder-tree-branch__load-more" onClick={() => this.props.onLoadMore(this.props.parentLocation)}>


Please, create a class method for handling onClick it wil prevent additional re-renderings on each update.

sunpietro · 2018-07-18T12:29:17Z

...universal-discovery/components/select-content-button/css/select.content.button.component.css

+.c-select-content-button {
+	position: absolute;
+	right: 1.3rem;
+	top: 50%;


It shouldn't be here. Because it's an information about block position. It should go to parent container style definitions.

sunpietro · 2018-07-18T12:30:11Z

...ules/universal-discovery/components/select-content-button/select.content.button.component.js

+    }
+
+    handleSelect(event) {
+        event.stopPropagation();


Why not preventDefault()?

Because I don't want to bubble this event to the parent.

sunpietro · 2018-07-18T12:31:36Z

src/modules/universal-discovery/universal.discovery.module.js

@@ -175,6 +187,13 @@ export default class UniversalDiscoveryModule extends Component {
        this.props.onConfirm(this.addContentTypeInfo(this.state.selectedContent));
    }

+    handleSingleConfirm() {
+        this.setState(
+            (state) => ({ ...state, selectedContent: [this.state.contentMeta] }),


(state) => ({ selectedContent: [state.contentMeta] }),

lserwatka · 2018-07-18T12:37:14Z

@dew326 did you cover bookmarks as well?

dew326 · 2018-07-18T12:39:30Z

@lserwatka Yes, I covered everything. Browse, Search, Create, Bookmarks in both single and multiple selection. Just didn't want to attach too many screenshots.

lserwatka · 2018-07-18T12:40:15Z

@dew326 ok, clear. Well done!

sunpietro · 2018-07-19T09:51:11Z

src/modules/universal-discovery/universal.discovery.module.js

@@ -106,6 +108,16 @@ export default class UniversalDiscoveryModule extends Component {
        if (!!contentMeta && !isPreviewMetaReady) {
            this.props.loadContentInfo(this.props.restInfo, contentMeta.ContentInfo.Content._id, this.updateContentMetaWithCurrentVersion);
        }
+
+        if (!this.props.multiple) {
+            this.canSelectContent(contentMeta, (canSelectContent) => {


Do you think it would be a good idea to extract the callback into a method like setCanSelectContentState?

barbaragr · 2018-07-23T10:20:33Z

According to the specification UDW title should be:

‘Select content item(s)’ for multi selection
‘Select a content item’ for single selection
Now there is: Select content

Error message Cannot read property 'identifier' of undefined

configure vendor/ezsystems/ezplatform-admin-ui/src/bundle/Resources/config/universal_discovery_widget.yml: for default configuration in allowed_content_types type article
allowed_content_types: [article]
clear cache
create Cotent Type with ezojcetrelationlist
go to Content/Content structure and click Create and choose CT from previous step
click Select content - UDW is opened for a while, then it disappear and error message is shown: Cannot read property 'identifier' of undefined

Scenario 2.2.2.8 Content item’s name link from spec https://ezno.sharepoint.com/:w:/r/sites/ProductTeam/_layouts/15/Doc.aspx?sourcedoc=%7B137050e3-dd3f-4629-913f-e9e1f2c1dfce%7D&action=default doesn't work at all. Is it included in this PR's scope? Similar question for next scenario Content item’s parent name link - there is no parent name in search results...

barbaragr

While embedding a content item in a rich text field, when I launch the UDW to select a content item, title should be: Select a content **item** to embed and now there is: Select a content to embed (according to scenario 2.2.1.2 from https://ezno.sharepoint.com/:w:/r/sites/ProductTeam/_layouts/15/Doc.aspx?sourcedoc=%7B0623259a-d7fa-4365-9807-207e44697563%7D&action=default)
Scenario 2.2.1.3 Title of the UDW when adding an image should be Select an image and now there is Select an image to embed
Scenario 2.2.2.3 Disabling the ‘Confirm’ button - there is no tooltip for disabled Confirm button. Tooltips message required by scenario: The Content Type is not allowed for selection
Group of scenarios 2.2.3 - similar like in point 3 - is it included in this PR?

dew326 · 2018-07-23T11:47:58Z

All points with the incorrect title are out of the scope of this PR (1, 4, 5).
3, 7 - switching to browse while searching is also out of scope. This can be done with "large content structure"

Only 2 and 6 are valid.

dew326 · 2018-07-24T06:24:03Z

@barbaragr fixed

EZP-29182/EZP-29187: Improvements in Universal Discovery Widget singl…

53d42db

…e and multi selection

dew326 added the Improvement label Jul 18, 2018

dew326 self-assigned this Jul 18, 2018

dew326 requested review from sunpietro and tischsoic July 18, 2018 12:13

sunpietro suggested changes Jul 18, 2018

View reviewed changes

dew326 added 2 commits July 18, 2018 15:02

after review

4ef7689

CS

27d4996

tischsoic approved these changes Jul 19, 2018

View reviewed changes

sunpietro approved these changes Jul 19, 2018

View reviewed changes

move callback to method

0dc100e

dew326 added the Ready for QA label Jul 19, 2018

barbaragr self-assigned this Jul 19, 2018

barbaragr requested changes Jul 23, 2018

View reviewed changes

barbaragr removed the Ready for QA label Jul 23, 2018

fix

7ec8865

barbaragr approved these changes Jul 24, 2018

View reviewed changes

barbaragr added the QA approved label Jul 24, 2018

lserwatka merged commit 4facd5d into ezsystems:master Jul 25, 2018

dew326 deleted the udw-improvements branch July 26, 2018 06:21

mnocon mentioned this pull request Jul 26, 2018

EZP-29182/EZP-29187: Adjust tests for UDW selection changes ezsystems/ezplatform-admin-ui#579

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EZP-29182/EZP-29187: Improvements in Universal Discovery Widget single and multi selection #94

EZP-29182/EZP-29187: Improvements in Universal Discovery Widget single and multi selection #94

dew326 commented Jul 18, 2018

sunpietro Jul 18, 2018

dew326 Jul 18, 2018

sunpietro Jul 18, 2018

sunpietro Jul 18, 2018

sunpietro Jul 18, 2018

dew326 Jul 18, 2018

sunpietro Jul 18, 2018

lserwatka commented Jul 18, 2018

dew326 commented Jul 18, 2018 •

edited

lserwatka commented Jul 18, 2018

sunpietro Jul 19, 2018

barbaragr commented Jul 23, 2018

barbaragr left a comment

dew326 commented Jul 23, 2018

dew326 commented Jul 24, 2018

EZP-29182/EZP-29187: Improvements in Universal Discovery Widget single and multi selection #94

EZP-29182/EZP-29187: Improvements in Universal Discovery Widget single and multi selection #94

Conversation

dew326 commented Jul 18, 2018

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

lserwatka commented Jul 18, 2018

dew326 commented Jul 18, 2018 • edited

lserwatka commented Jul 18, 2018

Choose a reason for hiding this comment

barbaragr commented Jul 23, 2018

barbaragr left a comment

Choose a reason for hiding this comment

dew326 commented Jul 23, 2018

dew326 commented Jul 24, 2018

dew326 commented Jul 18, 2018 •

edited