fix: fix "ifDefined" to update DOM only when attrs are changed (#415)

* Background: "ifThruthy" removes an attribute key and value if value is falsy, but it makes the templates less efficient, as lit-html updates attributes no matter, if the value is changed which causes some issues.
* Fixes: #401
* Changes:
* Rename "ifThruthy" to "ifDefined" and correct the directive logic to update only if the result of the directive is different from the previous value.
* Attributes undefined, null and empty strings are removed from DOM.
* Remove obsolete "common" partials
* Make use of boolean props syntax in lit-html
* Fix  default value for string props, the default becomes empty string or the the value, given via the defaultValue metadata key.
* Clean templates from non-existing component properties (tooltip, dir..)
* Remove type: "Active" everywhere the ui5-li is used, as it is "Active" by default

packages/base/src/State.js

@@ -31,12 +31,16 @@ class State {
 						return this._data[prop];
 					}
 
+					const propDefaultValue = propData.defaultValue;
+
 					if (propData.type === Boolean) {
 						return false;
+					} else if (propData.type === String) {  // eslint-disable-line
+						return propDefaultValue || "";
 					} else if (propData.multiple) { // eslint-disable-line
 						return [];
 					} else {
-						return propData.defaultValue;
+						return propDefaultValue;
 					}
 				},
 				set(value) {
@@ -125,19 +129,21 @@ class State {
 		// Initialize properties
 		const props = MetadataClass.getProperties();
 		for (const propName in props) { // eslint-disable-line
-
+			const propType = props[propName].type;
 			const propDefaultValue = props[propName].defaultValue;
 
-			if (props[propName].type === Boolean) {
+			if (propType === Boolean) {
 				defaultState[propName] = false;
 
 				if (propDefaultValue !== undefined) {
 					console.warn("The 'defaultValue' metadata key is ignored for all booleans properties, they would be initialized with 'false' by default"); // eslint-disable-line
 				}
 			} else if (props[propName].multiple) {
 				defaultState[propName] = [];
-			} else if (props[propName].type === Object) {
+			} else if (propType === Object) {
 				defaultState[propName] = "defaultValue" in props[propName] ? props[propName].defaultValue : {};
+			} else if (propType === String) {
+				defaultState[propName] = propDefaultValue || "";
 			} else {
 				defaultState[propName] = propDefaultValue;
 			}

packages/base/src/TemplateContext.js

@@ -22,7 +22,7 @@ const _convertSingleClass = (oClass, customStyleClasses) => {
 
 const _convertStyles = function _convertStyles(styles) {
 	if (!styles) {
-		return;
+		return "";
 	}
 
 	for (const i in styles) { // eslint-disable-line
@@ -34,10 +34,10 @@ const _convertStyles = function _convertStyles(styles) {
 				result.push(`${key}: ${stylesNs[key]}`);
 			}
 		});
-		styles[i] = result.length ? result.join("; ") : undefined;
+		styles[i] = result.length ? result.join("; ") : "";
 	}
 
-	return styles;
+	return styles || "";
 };
 
 class TemplateContext {

packages/base/src/renderer/ifDefined.js

@@ -0,0 +1,21 @@
+import {
+	AttributePart,
+	directive,
+	noChange,
+} from "lit-html";
+
+/*
+	lit-html directive that removes and attribute if it is undefined
+*/
+export default directive(value => part => {
+	if ((value === undefined || value === null || value === "") && part instanceof AttributePart) {
+		if (value !== part.value) {
+			const name = part.committer.name;
+			part.committer.element.removeAttribute(name);
+		}
+	} else if (part.committer && part.committer.element && part.committer.element.getAttribute(part.committer.name) === value) {
+		part.setValue(noChange);
+	} else {
+		part.setValue(value);
+	}
+});

packages/main/lib/hbs2lit/src/commonPartials.js

@@ -2,17 +2,8 @@ var aPartials = [`
 {{#*inline "controlData"}}
 	id="{{ctr._id}}"
 	data-sap-ui="{{ctr._id}}"
-	aria-hidden="{{ariaHidden}}"
-{{/inline}}`, `
-{{#*inline "accAttributes"}}
-	aria-readonly="{{ctr.readonly}}"
-	aria-disabled="{{ctr.disabled}}"
-	aria-hidden="{{ctr.hidden}}"
-	aria-required="{{ctr.required}}"
-	aria-selected="{{ctr.selected}}"
-	aria-checked="{{ctr.checked}}"
-	aria-describedby="{{ctr.describedby}}"
-	aria-labelledby="{{ctr.labelledby}}"
-{{/inline}}`];
+	?aria-hidden="{{ariaHidden}}"
+{{/inline}}`
+];
 
 module.exports = aPartials;

packages/main/lib/hbs2lit/src/litVisitor2.js

@@ -59,7 +59,7 @@ HTMLLitVisitor.prototype.MustacheStatement = function(mustache) {
 		this.blocks[this.currentKey()] += "${index}";
 	} else {
 		const path = normalizePath.call(this, mustache.path.original);
-		this.blocks[this.currentKey()] += "${ifTruthy(" + path + ")}";
+		this.blocks[this.currentKey()] += "${ifDefined(" + path + ")}";
 	}
 };
 

packages/main/lib/hbs2ui5/RenderTemplates/LitRenderer.js

@@ -1,7 +1,7 @@
 const buildRenderer = (controlName, litTemplate) => {
 	return `
-/* eslint no-unused-vars: 0 */	
-import ifTruthy from '@ui5/webcomponents-base/src/renderer/ifTruthy.js';
+/* eslint no-unused-vars: 0 */
+import ifDefined from '@ui5/webcomponents-base/src/renderer/ifDefined.js';
 import { html, svg, repeat } from '@ui5/webcomponents-base/src/renderer/LitRenderer.js';
 const ${controlName}LitRenderer = {};
 ${litTemplate}

packages/main/src/Button.hbs

@@ -2,9 +2,8 @@
 		type="button"
 		class="{{classes.main}}"
 		style="{{styles.main}}"
-		disabled="{{ctr.disabled}}"
-		aria-disabled="{{ariaDisabled}}"
-		title="{{ctr.tooltip}}"
+		?disabled="{{ctr.disabled}}"
+		?aria-disabled="{{ariaDisabled}}"
 		tabindex="{{tabindex}}"
 		data-sap-focus-ref
 		{{> ariaPressed}}
@@ -17,7 +16,7 @@
 		{{/if}}
 
 		{{#if ctr.text.length}}
-			<span id="{{ctr._id}}-content" dir="{{dir}}" class="{{classes.text}}">
+			<span id="{{ctr._id}}-content" class="{{classes.text}}">
 				<bdi>
 					<slot></slot>
 				</bdi>

packages/main/src/ButtonTemplateContext.js

@@ -11,6 +11,8 @@ class ButtonTemplateContext {
 				},
 			},
 			styles: {
+				main: {
+				},
 			},
 			iconSrc: state._active ? state.activeIcon : state.icon,
 			ariaDisabled: state.disabled ? "true" : undefined,

packages/main/src/Calendar.js

@@ -45,6 +45,7 @@ const metadata = {
 		/**
 		 * Sets a calendar type used for display.
 		 * If not set, the calendar type of the global configuration is used.
+		 * Available options are: "Gregorian", "Islamic", "Japanese", "Buddhist" and "Persian".
 		 * @type {string}
 		 * @public
 		 */

packages/main/src/Card.js

@@ -36,32 +36,32 @@ const metadata = {
 
 		/**
 		 * Defines the title displayed in the <code>ui5-card</code> header.
-		 * @type {String}
+		 * @type {string}
+		 * @defaultvalue ""
 		 * @public
 		 */
 		heading: {
 			type: String,
-			defaultValue: "",
 		},
 
 		/**
 		 * Defines the subtitle displayed in the <code>ui5-card</code> header.
-		 * @type {String}
+		 * @type {string}
+		 * @defaultvalue ""
 		 * @public
 		 */
 		subtitle: {
 			type: String,
-			defaultValue: "",
 		},
 
 		/**
 		 * Defines the status displayed in the <code>ui5-card</code> header.
-		 * @type {String}
+		 * @type {string}
+		 * @defaultvalue ""
 		 * @public
 		 */
 		status: {
 			type: String,
-			defaultValue: "",
 		},
 
 		/**

packages/main/src/CheckBox.hbs

@@ -3,7 +3,7 @@
 	style="{{styles.main}}"
 	role="checkbox"
 	aria-checked="{{ctr.checked}}"
-	aria-readonly="{{ariaReadonly}}"
+	?aria-readonly="{{ariaReadonly}}"
 	tabindex="{{tabIndex}}">
 		<div id="{{ctr._id}}-CbBg" class="{{classes.inner}}">
 			<input id="{{ctr._id}}-CB" type='checkbox' ?checked="{{ctr.checked}}" ?readonly="{{ctr.readonly}}" data-sap-no-tab-ref/>

packages/main/src/CheckBox.js

@@ -70,7 +70,6 @@ const metadata = {
 		 * @public
 		 */
 		text: {
-			defaultValue: "",
 			type: String,
 		},
 
@@ -80,12 +79,12 @@ const metadata = {
 		 * <b>Note:</b> Available options are <code>Warning</code>, <code>Error</code>, and <code>None</code> (default).
 		 *
 		 * @type {string}
-		 * @defaultvalue None
+		 * @defaultvalue "None"
 		 * @public
 		 */
 		valueState: {
-			defaultValue: ValueState.None,
 			type: ValueState,
+			defaultValue: ValueState.None,
 		},
 
 		/**
@@ -111,7 +110,8 @@ const metadata = {
 		 * will be created inside the <code>ui5-checkbox</code> so that it can be submitted as
 		 * part of an HTML form. Do not use this property unless you need to submit a form.
 		 *
-		 * @type {String}
+		 * @type {string}
+		 * @defaultvalue ""
 		 * @public
 		 */
 		name: {

packages/main/src/DatePicker.js

@@ -41,11 +41,11 @@ const metadata = {
 		 * Defines a formatted date value.
 		 *
 		 * @type {string}
+		 * @defaultvalue ""
 		 * @public
 		 */
 		value: {
 			type: String,
-			defaultValue: "",
 		},
 
 		/**
@@ -54,6 +54,7 @@ const metadata = {
 		 * <code>Success</code>.
 		 *
 		 * @type {string}
+		 * @defaultvalue "None"
 		 * @public
 		 */
 		valueState: {
@@ -65,6 +66,7 @@ const metadata = {
 		 * Determines the format, displayed in the input field.
 		 *
 		 * @type {string}
+		 * @defaultvalue ""
 		 * @public
 		 */
 		formatPattern: {
@@ -74,7 +76,7 @@ const metadata = {
 		/**
 		 * Determines the calendar type.
 		 * The input value is formated according to the calendar type and the picker shows
-		 * months and years from the specified calendar.
+		 * months and years from the specified calendar. Available options are: "Gregorian", "Islamic", "Japanese", "Buddhist" and "Persian".
 		 *
 		 * @type {string}
 		 * @public
@@ -111,10 +113,10 @@ const metadata = {
 		 * <br><br>
 		 * <b>Note:</b> The placeholder is not supported in IE. If the placeholder is provided, it won`t be displayed in IE.
 		 * @type {string}
+		 * @defaultvalue ""
 		 * @public
 		 */
 		placeholder: {
-			defaultValue: null,
 			type: String,
 		},
 
@@ -128,7 +130,8 @@ const metadata = {
 		 * will be created inside the <code>ui5-datepicker</code> so that it can be submitted as
 		 * part of an HTML form. Do not use this property unless you need to submit a form.
 		 *
-		 * @type {String}
+		 * @type {string}
+		 * @defaultvalue ""
 		 * @public
 		 */
 		name: {

packages/main/src/Icon.js

@@ -28,7 +28,7 @@ const metadata = {
 		 * <br>
 		 * <code>src='sap-icons://add'</code>, <code>src='sap-icons://delete'</code>, <code>src='sap-icons://employee'</code>.
 		 *
-		 * @type {String}
+		 * @type {string}
 		 * @public
 		*/
 		src: { type: URI, defaultValue: null },

packages/main/src/IconTemplateContext.js

@@ -12,8 +12,8 @@ class IconTemplateContext {
 			ctr: state,
 			iconContent: iconInfo.content,
 			role: state._customAttributes.role || role,
-			ariaExpanded: state._customAttributes["aria-expanded"],
-			ariaLabelledBy: state._customAttributes["aria-labelledby"],
+			ariaExpanded: state._customAttributes["aria-expanded"] || "",
+			ariaLabelledBy: state._customAttributes["aria-labelledby"] || "",
 			classes: {
 				main: IconTemplateContext.getMainClasses(state, iconInfo),
 			},

packages/main/src/Input.hbs

@@ -1,8 +1,7 @@
 <div {{> controlData}}
 	class="{{classes.main}}"
 	style="{{styles.main}}"
-	aria-invalid="{{ariaInvalid}}"
-	aria-labelledBy="{{ariaLabelledBy}}"
+	?aria-invalid="{{ariaInvalid}}"
 >
 <div id="{{ctr._id}}-wrapper"
 	class="{{classes.wrapper}}"