ampproject / amphtml

The AMP web component framework.
https://amp.dev
Apache License 2.0
14.89k stars 3.88k forks source link

[amp-truncate-text:1.0] Bento tracking issue #31685

Open rcebulko opened 3 years ago

rcebulko commented 3 years ago
(## High-level requirements Consideration Ready when Status
Component compatibility Conformance with 0.1 (see below) ✖️
Testing Unit tests and e2e tests from 0.1 ✖️
Internationalization n/a
Analytics n/a n/a
Accessibility Audit performed, [[Reference from 0.1?]] ✖️
Page experience Audit performed ✖️
Documentation Written ✖️
Storybook Samples written ✖️

Component compatibility

Attributes to support: [none (?)]

Migration Notes:

[none yet]

Open tasks:

rcebulko commented 3 years ago

/cc @caroqliu Does this seem like a good starting point? I didn't see any other attributes other than extended-emp-global which don't seem to need explicit support mentioned in these issues.

Related note: would it be helpful for me to throw together an issue template for Bento tracking issues like these? I noticed it's not yet in the list of options when creating a new issue

caroqliu commented 3 years ago

/cc @caroqliu Does this seem like a good starting point? I didn't see any other attributes other than extended-emp-global which don't seem to need explicit support mentioned in these issues.

Yeah, looks good. Though overflow-styles doesn't seem to be implemented in 0.1 unless I'm missing something? I see it in the validation rules but not sure how it is used otherwise.

Maybe?: Support passing in a parser callback for PreactBaseElement props This was a necessary step in amp-fit-text that seemed like it may be relevant

This is supported now as the parseAttrs property in an AMP component's ['props'] definition. Ex. here for how Timeago uses it: https://github.com/ampproject/amphtml/blob/e212306a22a3347b7e780a77226cf87ed5d26dc5/extensions/amp-timeago/1.0/amp-timeago.js#L53-L57

By default if a prop has a type specified, like boolean, then PreactBaseElement will parse it by default to getAttribute with the attr specified, and call Boolean() on that value.

Sometimes components will have component-specific processing needs for attributes beyond what we want to stuff into PreactBaseElement - things like parseDateAttrs that maybe don't need to be supported by default since only a few specialized components would even use it.

For amp-fit-text, we wanted to cast its attr values to Number type before the above was implemented in PreactBaseElement. You likely will not need to use this for this component because it doesn't have any fancy attributes that I'm aware of.

Related note: would it be helpful for me to throw together an issue template for Bento tracking issues like these? I noticed it's not yet in the list of options when creating a new issue

That would be super helpful, good idea!

Internationalization ❓

Mostly this is about RTL support (should not be needed here as children passed to this component should respect the document's dir setting by default. If there are any hard coded strings in English it would be a bonus if we had attributes to configure them in other languages, but that doesn't appear to be relevant here either.