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

feat(timeline): Content is optional and title mandatory - FRONT-2036 #598

Merged
merged 2 commits into from
Feb 22, 2021

Conversation

planctus
Copy link
Contributor

PR description

Please drop a few lines about the PR: what it does, how to test it, etc.

QA Checklist

In order to ensure a safe and quick review, please check that your PR follow those guidelines:

  • I have put the vanilla component as devDependencies
  • I have put the specs package as devDependencies
  • I have added the components directly used in the twig file (with include or embed) as dependencies
  • My component is listed in @ecl-twig/ec-components's dependencies
  • My variables naming follow the guidelines (snake case for twig)
  • I have provided tests
  • I have provided documentation (for the "notes" tab)
  • If my local yarn.lock contains changes, I have committed it
  • I have given my PR the proper label (pr: review needed to indicate that I'm done and now waiting for a review ,pr: wip to indicate that I'm actively working on it ...)

@planctus planctus added pr: review needed Use this label to show that your PR needs to be review tag: bug fix labels Feb 16, 2021
@emeryro emeryro self-assigned this Feb 17, 2021
@emeryro emeryro added pr: under review and removed pr: review needed Use this label to show that your PR needs to be review labels Feb 17, 2021
- "label": (string) (default: '')
- "title": (string) (default: '')
- "content": (block) (default: '')
- "content": (string) (optional) (default: '')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't it be an issue to define the content as a string only?
According to the usage page, users can put other type of content in it (media for instance)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, i'd say that in these docs on all our templates we refer to the data type, whatever this variable will contain it's going to be used as a string, not an array, not an object, not a boolean, in other words. But you are right that maybe we should treat this, as the other variables that are "allowed" or supposed to contain html (the ones that we were rendering with |raw) in a different way, because string in those cases might be confused with "plain text", we could say something like html markup marked as a safe string to somehow specify that it would be otherwise escaped in an environment with the twig autoescape on.

@planctus planctus added pr: review needed Use this label to show that your PR needs to be review and removed pr: questions labels Feb 18, 2021
@emeryro emeryro removed the pr: review needed Use this label to show that your PR needs to be review label Feb 22, 2021
@emeryro emeryro merged commit 887ea56 into develop Feb 22, 2021
@emeryro emeryro deleted the front-2036 branch February 22, 2021 12:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants