Fix/basics

[trollkotze]

1. [Core]: allow HTMLElement as template

In the Riba documentation it says:

A component object must define a template function, which returns the template for the component (this can be an HTML string or the actual element). It must also define an initialize function, which returns the scope object to bind the view with (this will likely be a controller / viewmodel / presenter).

(emphasis added)

But to use an actual HTML element as a return value was actually not possible before. So I added the possibility.

2. [utils] fix extend, concat and clone methods ... [Utils] fix error in previous fix for extend method

• The extend method in packages/utils/src/type.ts, originally copied and modified from this blog was making a wrong existence check: checking for a truthy value instead of for existence of a property. Using Object.hasOwnProperty, as in the original source, fixes this. (When copying code from elsewhere, I think it is good to always add a reference, for attribution and courtesy as well as to be able to check if it was really copied correctly when it causes errors.)

• The concat method was unncessarily calling extend(deep, object1, object1, object2), instead of just extend(deep, object1, object2), causing some overhead (and possibly even bugs when deep-cloning one object to itself, not sure...). This was fixed. (The concat method is actually the same as extend, but called with only two object parameters. It is redundant.)

• The clone method did not deep-clone arrays when called with deep = true. Now it does.

3. [Bs4] fix nullcheck bug in TemplatesComponent: allow attributes to be "0" or "false" values.

Again, an incorrect existence check, treating attributes like "0" or "false" as not existing. Fixed.

4. [Core] Fix nullcheck bug in Component.checkRequiredAttributes: recognize falsy attributes like 0 or false if they are set.

Again, an incorrect existence check, treating attributes like "0" or "false" as not existing. Fixed.

5. [Bs4] fix newly added Tooltip and Popover services

TooltipService and PopoverService, ported over from Boostrap 5 Alpha, had some type and logic errors which were fixed.

6. [Bs4] fix TemplatesComponent: check if attribute is required before logging error

The TemplatesComponent in the Bootstrap4 package checks for the existence of attributes on the templates and errors out when some are not set. But it ignored the "required" property for those attributes, logging errors (and returning early, omitting operations thereafter) even for missing attributes that are not required.

[trollkotze]

Sieht eigentlich ganz gut aus, hattest du getestet ob du etwas kaputt gemacht hast? Sonst kann ich das aber auch nochmal machen.

Ich weiß nur, dass ich was heile gemacht hab. In der Slideshow-Component gab's vorher Fehler. Die zusätzliche Möglichkeit, in der Component.template()-Methode ein HTMLElement auszuspucken, statt eines Strings, sollte nichts kaputt machen. Das wird in Component.loadTemplate() dann weiter verarbeitet je nach Typ. Unkompliziert.

[dnil-io]

Du hattest ja etwas bei den Attributen geändert im ersten Commit, und an einer Stelle glaube ich getAttribute genommen statt irgendeine Riba Methode, da war ich mir etwas unsicher aber wenn die noch funktionieren, dann passt alles 👍

On Wed, Aug 12, 2020, 13:54 Moritz Raguschat notifications@github.com wrote:

Sieht eigentlich ganz gut aus, hattest du getestet ob du etwas kaputt gemacht hast? Sonst kann ich das aber auch nochmal machen.

Ich weiß nur, dass ich was heile gemacht hab. In der Slideshow-Component gab's vorher Fehler. Die zusätzliche Möglichkeit, in der Component.template()-Methode ein HTMLElement auszuspucken, statt eines Strings, sollte nichts kaputt machen. Das wird in Component.loadTemplate() dann weiter verarbeitet je nach Typ. Unkompliziert.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ribajs/riba/pull/18#issuecomment-672826087, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE7IULXTTC6YVPUXMA7IONDSAJ7I7ANCNFSM4P4KQ6AQ .

[trollkotze]

Du hattest ja etwas bei den Attributen geändert im ersten Commit, und an einer Stelle glaube ich getAttribute genommen statt irgendeine Riba Methode, da war ich mir etwas unsicher aber wenn die noch funktionieren, dann passt alles +1

Vermutlich meinst du das hier (weil das die einzige Stelle im Diff ist, wo getAttribute vorkommt).

Im Original:

    const attrValue = this.transformTemplateAttribute(
        attribute.name,
        tpl.getAttribute(attribute.name)
      );
      if (attribute.required && !attrValue) {
        // ... Fehler
      }
       attributes[camelCase(attribute.name)] = attrValue;
    }

Wie du siehst, kommt getAttribute dort auch im Original vor. Und ich ersetzte nicht irgendetwas durch getAttribute, sondern ich rufe nur this.transformTemplateAttribute ein paar Zeilen später auf, nachdem ich erst mal das Zwischenergebnis von getAttribute gecheckt habe (anstatt hinterher das Ergebnis von this.transformTemplateAttribute zu checken). Weil der Check hier einfacher ist: Wenn typeof getAttribute(...) === "string", dann ist das Attribut gesetzt (und wird von transformTemplateAttribute danach in boolean oder number umgewandelt oder so gelassen).

Das Problem im Original war if (attribute.required && !attrValue) {.

attrValue konnte z.B. 0 (number) oder false (boolean) sein. Dann wäre der Check hier fehlgeschlagen (!attrValue wäre false), der eigentlich nur testen sollte, ob das Attribut überhaupt gesetzt ist oder nicht (auch wenn es von transformTemplateAttribute dann zu einem falsy-Wert wie false oder 0 geparset würde).

Jetzt ist der Check statt desen if (typeof attrValue !== "string" && attribute.required) { Und attrValue wird erst danach mit transformTemplateAttribute geparset.

aber wenn die noch funktionieren, dann passt alles +1

Funktionieren tut hier jetzt alles. Auch dass man einem Template in einer TemplatesComponent jetzt Attribute wie jochen="false" oder hastenichgesehn="0" übergeben kann, wenn diese Attribute als "required" verlangt werden. Was vorher zu einem Fehler geführt hätte.

Anmerkung: Solche fehlerhaften Null-Checks (d.h. so etwas wie if (obj[property]), wenn eigentlich nur geprüft werden soll, ob obj[property] gesetzt ist - auch wenn ein Wert wie 0 oder false (oder gar null oder undefined, je nach Kontext) valide wäre) kommen wahrscheinlich im Repo noch öfter vor. Hatte ja noch zwei andere gefixt.

[dnil-io]

Jaa, das ist ja super soweit. Dachte nur das die transformTemplateAttribute vielleicht auch für sowas wie rv-jochen zuständig ist. Aber wenn es so passt ist ja super.