adhocracy3 icon indicating copy to clipboard operation
adhocracy3 copied to clipboard

Widget parameter noheader cannot be set to false

Open 2e2a opened this issue 8 years ago • 2 comments

When having data-noheader="false" in the embed-code the header is not shown.

Only removing the data-noheader parameter enables the header.

2e2a avatar Oct 05 '16 11:10 2e2a

This is not properly documented, but it is actually how the embed API works: If the attribute is specified the value is true, if it is not specified the value is false. This is consistent with boolean attributes in HTML.

However, this mechanism is only used for nocenter and noheader. autoresize and autourl are converted to boolean by jquery. The defaults are true for autoresize and false for autourl.

So I see two issues here:

  • The documentation is not clear
  • The behavior is inconsistent

Fixing this inconsistency would actually break a public API, so we should be very careful about it. I see three options:

  1. Changing the behavior of autoresize and autourl. This would be a big problem because the usage of autoresize="false" is actually recommended for plain embeds in the docs. (value = data.hasOwnProperty("key"))
  2. Changing the behavior of nocenter and noheader. This would be a smaller issue: The most probable issue is that the attribute might have been used without a value or with an empty value. It is quite possible that this is not the case. However, the implementation would be difficult because we generally avoid using jquery. (value = $element.data("key"))
  3. Changing both. Any case except for no attribute or a value of false would be interpreted as true. This would only break the autoresize default value. (value = data.hasOwnProperty("key") && data["key"] !== "false")

I am not happy with any of these solutions. If I had to choose it would be either 2 or 3.

xi avatar Oct 11 '16 16:10 xi

I am in favor of 2, as it would make the parameter usage consistent and would break less than 1.

2e2a avatar Oct 12 '16 07:10 2e2a