feat(ui5-upload-collection): implement new webcomponent #1316

dimovpetar · 2020-03-19T09:24:34Z

ui5-upload-collection is new webcomponent located in fiori package, which benefits and brings some of ui5-list features, but it shows different items of type ui5-upload-collection-item.
It is in the responsibility of the app developer to put ui5-file-uploader in the header, create and add new ui5-upload-collection-item(s) to the collection.

Main features:

header slot
items slot (default)
items have special visualisation, suitable for files representation
items have "file" property, to allow app developers to set and get file object
drag and drop overlay
progress indicator

ui5-upload-collection is new webcomponent located in fiori package, which benefits and brings some of ui5-list features, but it shows different items of type ui5-upload-collection-item. It is in the responsibility of the app developer to put ui5-file-uploader in the header, create and add new ui5-upload-collection-item(s) to the collection. Main features: - header slot - items slot (default) - items have special visualisation, suitable for files representation - items have "file" property, to allow app developers to set and get file object - drag and drop overlay

…collection

packages/fiori/src/UploadCollection.hbs

vladitasev · 2020-03-24T11:59:37Z

packages/fiori/src/UploadCollection.js

+ * Ensures that there is only 1 listener per type attached (drag, drop, leave). Event listeners will be only attached when
+ * there is at least 1 UploadCollection registered in the set.
+ */
+const bodyDnDHandler = {


Please move this to a separate module in main/src/upload-utils/ f.e. or whatever name you like

main/src of fiori/src?

packages/fiori/src/UploadCollection.js

vladitasev · 2020-03-24T12:05:33Z

packages/fiori/src/UploadCollectionItem.hbs

@@ -0,0 +1,35 @@
+{{>include "../../main/src/ListItem.hbs"}}


Not sure how we can proceed here. We need to import a template from another package
@ilhan007 @pskelin @MapTo0 @fifoosid
Opinions?
Should we modify the plugin to understand node_modules/ and write here a full @ui5/webcomponents/dist..../ path?

vladitasev · 2020-03-24T12:06:53Z

packages/fiori/src/UploadCollectionItem.js

+
+	onAfterRendering() {
+		if (this.focused && this._editing) {
+			const inp = this.shadowRoot.getElementById("ui5-uci-edit-input");


Can you use our declarative focus dom ref and call focus()?

vladitasev · 2020-03-24T12:07:44Z

packages/fiori/src/UploadCollectionItem.js

+	}
+
+	get _fileNameWithoutExtension() {
+		return this.fileName.split(".")[0];


Please create a more robust check, file may have dots inside the name too

vladitasev · 2020-03-24T12:11:09Z

Does not render correctly on IE11

dimovpetar · 2020-03-24T13:42:17Z

Does not render correctly on IE11

Fixed. They were caused by flex.

…collection

vladitasev

Looks really good!

use events for body dnd class
create a new message bundle
bind all events in the template

vladitasev · 2020-04-01T09:19:28Z

packages/fiori/src/upload-utils/BodyDragAndDrop.js

@@ -0,0 +1,79 @@
+


This is upload collection related. Please put that in the name somehow

Renamed it to UploadCollectionBodyDnD

vladitasev · 2020-04-01T09:20:55Z

packages/fiori/src/upload-utils/BodyDragAndDrop.js

+
+	lastDragEnter = event.target;
+	uploadCollections.forEach(uc => {
+		if (uc._dndOverlayMode !== UploadCollectionDnDOverlayMode.Drop) {


The right pattern for this kind of behavior is: publish/subscribe. This file should not touch the private APIs of upload collections. They should register events and have this file fire the events, and they should do the work themselves.
See EventProvider.js in the base project. Use that as in OpenUI5

As a result, the methods this will export will be called something like attachDnDHandler, detachDnDHandler rather than add/remove upload collection instance

packages/fiori/src/UploadCollection.js

vladitasev · 2020-04-01T09:25:18Z

packages/fiori/src/UploadCollection.js

+		this._removeDragAndDropListeners();
+	}
+
+	_addDragAndDropListeners() {


You should not register event listeners unless absolutely necessary. You should only bind events in the template and let lit handle the lifecycle. If you need to bind the events conditionally, just bind them and return early in the handlers if the condition is not met. Thus the whole onbefore/onafter rendering logic should go away

Done. I was able to simplify the logic even further by adding the subscriptions not on the root div, but on the dedicated div for drag and drop overlay

vladitasev · 2020-04-01T09:28:43Z

packages/fiori/src/UploadCollection.js

+			Label.define(),
+			List.define(),
+			Title.define(),
+			fetchI18nBundle("@ui5/webcomponents"),


"-fiori"
Each package should use its own bundle. These will be the first translations for this package :) so you get to create it

…collection

packages/fiori/src/UploadCollection.js

ilhan007 · 2020-04-02T16:51:42Z

packages/fiori/src/UploadCollectionItem.js

+import litRender from "@ui5/webcomponents-base/dist/renderer/LitRenderer.js";
+import { fetchI18nBundle, getI18nBundle } from "@ui5/webcomponents-base/dist/i18nBundle.js";
+import Button from "@ui5/webcomponents/dist/Button.js";
+import Icon from "@ui5/webcomponents/dist/Icon.js";


if you don't have <ui5-icon tag in UploadCollectionitem.hbs, you don't need to import Icon.js

ilhan007 · 2020-04-02T16:52:41Z

packages/fiori/src/UploadCollectionItem.js

+const metadata = {
+	tag: "ui5-upload-collection-item",
+	properties: /** @lends sap.ui.webcomponents.fiori.UploadCollectionItem.prototype */ {
+		/**


make sure all the public properties have public, type and defaultvalue tags

ilhan007 · 2020-04-02T16:53:03Z

packages/fiori/src/UploadCollectionItem.js

+ *
+ * @constructor
+ * @author SAP SE
+ * @alias sap.ui.webcomponents.fiori.UploadCollectionItem


add since 1.0.0-=rc.7

ilhan007 · 2020-04-02T16:55:17Z

packages/fiori/src/themes/UploadCollection.css

@@ -0,0 +1,123 @@
+:host {


consider the "hidden" attribute, use some of the existing components for reference, but in general if the someone sets "hidden" to the component (to the host) the component should hide

ilhan007 · 2020-04-02T16:58:18Z

packages/fiori/src/types/UploadCollectionDnDMode.js

@@ -0,0 +1,29 @@
+import DataType from "@ui5/webcomponents-base/dist/types/DataType.js";
+
+/**


The enums should be documented as well. Use the AvatarShape.js for reference and it is ok to be public

I added jsdoc to this type, but it is only used in a private property "_dndOverlayMode" and thats why I kept it private

ilhan007 · 2020-04-02T16:58:28Z

packages/fiori/src/types/UploadState.js

+import DataType from "@ui5/webcomponents-base/dist/types/DataType.js";
+
+const UploadStates = {
+	/**


same as previous

vladitasev · 2020-04-03T07:01:16Z

packages/fiori/src/UploadCollectionItem.js

+	}
+
+	get _progressText() {
+		switch (this.uploadState) {


IMO switch is an overkill here:

`if (error) {
return "...";
}

if (warning) {
return "...";
}

return... "";`

If you insist on switch it's fine, just fix tabulation

ESLint doesn't let me fix the indentation, but you are right that it is too much to use switch-case here. I have turned it to ifs

vladitasev · 2020-04-03T07:03:37Z

packages/fiori/src/UploadCollectionItem.js

+	}
+
+	get _fileExtension() {
+		return this.fileName.includes(".") ? `.${this.fileName.split(".").pop()}` : "";


this will have a problem with files such as .gitignore that start with a dot and have no extension
Please research how to get the extension of a file safely and once you find the algorithm, implement it in: base/src/utils/getFileExtension.js
and let it export a single method
export default getFileExtension;
We might need it elsewhere in the future too :)

ilhan007 · 2020-04-03T12:13:34Z

packages/fiori/src/types/UploadState.js

+import DataType from "@ui5/webcomponents-base/dist/types/DataType.js";
+
+/**
+ * Different types of AvatarShape.


typo AvatarShape

vladitasev · 2020-04-03T13:52:42Z

packages/fiori/test/samples/UploadCollection.sample.html

@@ -94,21 +75,22 @@ <h3>UploadCollection</h3>
 				return uci;
 			}

-			fileUploader.addEventListener("ui5-change", function(event) {
-				var files = event.detail.files;
+			fileUploader.addEventListener("ui5-change", event => {


I don't think we transpile samples pages and this will not work on IE.

Fixed it, now only the displayed code is in es6

outdated

ilhan007

just the code that is displayed to the user should be es6 if possible

dimovpetar added 15 commits March 19, 2020 11:14

add property "file" to UploadCollectionItem

911bb3f

remove icon and image properties and add thumbnail slot

eac9c13

make file name clickable if property is set

106aa55

add withDnd property

34f352d

rename withDnd property to noDnd

eaa8df7

rename property deleteDisabled to noDelete

aa7dda3

add edit functionality

e6a4207

Merge branch 'master' of github.com:SAP/ui5-webcomponents into upload…

eaa23d6

…collection

add event listeners for drag and drop on the body only once

96c2ba0

Merge branch 'master' of github.com:SAP/ui5-webcomponents into upload…

2690d0c

…collection

make all texts translatable

56a666f

add missing css vars

7cdba4d

make borders correct in belize_hcb/hcw

cfda15b

added fileNameClick event on UploadCollectionItem

f7a16ee

vladitasev requested changes Mar 24, 2020

View reviewed changes

dimovpetar added 5 commits March 24, 2020 14:15

add jsdoc and missing dependencies

74331e0

move BodyDragAndDrop to fiori/src/upload-utils/BodyDragAndDrop.js

1309612

combined noDnd with return statements for readability

fbcaebe

fix BodyDragAndDrop import path and append file extension after edit

0605ac4

fix IE11 flex issues

066232a

dimovpetar added 7 commits March 24, 2020 16:02

improve _fileNameWithoutExtension method

6850bb0

use data-sap-focus-ref for focusing input

594440d

add tests

fd4c91d

rename uploadCollectionDemoHelper to uploadCollectionScript

f99c33e

use correct css vars for belize hcb/hcw

797d145

use correct css var for dnd icon and text

ae878d1

select input's text after edit button is pressed

b6f3157

add tests for progress indicator

2bc17ae

dimovpetar changed the title ~~WIP(ui5-upload-collection): implement new webcomponent~~ feat(ui5-upload-collection): implement new webcomponent Mar 26, 2020

dimovpetar added 4 commits March 27, 2020 11:50

fix overflow issues

062b138

make span for file extension inline-block

10eccec

add playground sample

aa7a75e

Merge branch 'master' of github.com:SAP/ui5-webcomponents into upload…

3883c6d

…collection

vladitasev requested changes Apr 1, 2020

View reviewed changes

dimovpetar added 7 commits April 1, 2020 13:39

use @ui5/webcomponents-fiori messagebundle

f59fe6a

subscribe UploadCollection's dnd listeners in the hbs

99d0c20

Merge branch 'master' of github.com:SAP/ui5-webcomponents into upload…

f052932

…collection

use publish-subscribe pattern for drag and drop events on the body

8b72a56

Merge branch 'master' of github.com:SAP/ui5-webcomponents into upload…

142a019

…collection

adopt global css vars

0cccd75

set correct opacities

a1f7099

ilhan007 requested changes Apr 2, 2020

View reviewed changes

vladitasev reviewed Apr 3, 2020

View reviewed changes

dimovpetar added 3 commits April 3, 2020 10:52

add missing jsdoc tags - defaultvalue, type, public

e6edcdf

consider hidden attribute

db596a8

introduce base/util/getFileExtension and consider hidden files

2eb644a

ilhan007 reviewed Apr 3, 2020

View reviewed changes

dimovpetar added 2 commits April 3, 2020 15:56

fix jsdoc typos and add new badge

43d2dfc

simplify playground sample

ef5cd45

vladitasev previously requested changes Apr 3, 2020

View reviewed changes

ilhan007 approved these changes Apr 3, 2020

View reviewed changes

ilhan007 requested changes Apr 3, 2020

View reviewed changes

remove es6 from snippets in the playground

a1d2918

ilhan007 approved these changes Apr 3, 2020

View reviewed changes

ilhan007 merged commit 54038e4 into SAP:master Apr 3, 2020

dimovpetar deleted the uploadcollection branch October 29, 2020 16:41

		@@ -0,0 +1,29 @@
		import DataType from "@ui5/webcomponents-base/dist/types/DataType.js";

		/**

feat(ui5-upload-collection): implement new webcomponent #1316

feat(ui5-upload-collection): implement new webcomponent #1316

Conversation

dimovpetar commented Mar 19, 2020 • 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

vladitasev commented Mar 24, 2020

dimovpetar commented Mar 24, 2020

vladitasev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dimovpetar Apr 1, 2020 • 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

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

vladitasev Apr 3, 2020 • 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

ilhan007 left a comment

Choose a reason for hiding this comment

dimovpetar commented Mar 19, 2020 •

edited

Loading

dimovpetar Apr 1, 2020 •

edited

Loading

vladitasev Apr 3, 2020 •

edited

Loading