Skip to content
This repository has been archived by the owner on Jun 20, 2023. It is now read-only.

Widget parameter noheader cannot be set to false #2772

Open
2e2a opened this issue Oct 5, 2016 · 2 comments
Open

Widget parameter noheader cannot be set to false #2772

2e2a opened this issue Oct 5, 2016 · 2 comments

Comments

@2e2a
Copy link
Contributor

2e2a commented Oct 5, 2016

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

Only removing the data-noheader parameter enables the header.

@2e2a 2e2a added the frontend label Oct 5, 2016
@der-john der-john self-assigned this Oct 11, 2016
@der-john der-john removed their assignment Oct 11, 2016
@xi
Copy link
Collaborator

xi commented Oct 11, 2016

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.

@2e2a
Copy link
Contributor Author

2e2a commented Oct 12, 2016

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants