search

facebookarchive / instant-articles-builder

Instant Articles Rules Editor
https://facebook.github.io/instant-articles-builder
Other
125 stars 67 forks source link

typo in rule-definitions.js #90

Closed manishbisht closed 3 years ago

manishbisht commented 6 years ago

Typo in Line 139: displayName: 'HRef',

I think 'R' should be small displayName: 'Href',

pestevez commented 6 years ago

Hey @manishbisht, thanks for your comment. To be honest, I am not sure which one is the correct one (since most of the references I found to href is all lowercase. In any case, since the displayName is only used to generate the text of a label, both options should be fine.

I am leaning towards something like 'Destination (href)' what do you think?

manishbisht commented 6 years ago

'Destination (href)' is also good. I think 'Hyperlink' will look much better.

pestevez commented 6 years ago

We are using 'Link' (which is closer to the one you are proposing) to describe the whole rule. In this case, I think 'HRef' (or a variation of it) describes accurately the attribute of the hyperlink that we want to set; but I am happy to discuss it with a broader group.

image

Perhaps clarifying the difference between the rule (for example, Hyperlink) and its properties (href, rel, etc.) is one of the areas where we should add some clarity.

manishbisht commented 6 years ago

URL can also be a good replacement. What do you think @pestevez ?

vaibhavsingh97 commented 6 years ago

@manishbisht @pestevez definitely URL is a good replacement, Should I go and fix the issue?

pestevez commented 3 years ago

Hi @manishbisht and @vaibhavsingh97 - apologies for the delayed response. URL looks good to me.

pestevez commented 3 years ago

Changed in #90. Closing now.