-
Notifications
You must be signed in to change notification settings - Fork 0
ADSDEV-2383: add styleType attribute to Link #114
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
9b2761d to
241d781
Compare
| interface Link extends Parent { | ||
| type: "link" | ||
| url: string | ||
| title: string | ||
| children: Phrasing[] | ||
| styleType?: 'onward-journey-link' | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Non-blocking)
I wonder if we need to start documenting all our properties to help other Engineers but also to avoid that the language and the meaning of properties drift to much.
This is more related to other work we are doing.
See also suggestion/thoughts below...
/**
* Represents a hyperlink element within rich text content.
* @interface Link
* @extends Parent
* @property {'link'} type - Identifies this node as a link.
* @property {string} url - The target URL of the link.
* @property {string} title - The link’s title or tooltip text.
* @property {Phrasing[]} children - The phrasing content inside the link.
*
* @property {LinkStyle} [style] - Optional style. Use to define a style applied to the element based on the purpose of it.
*
* @typedef {'recommendation'} A link that suggest to the user a follow-up document
*/
type LinkStyle = 'recommendation'
interface Link extends Parent {
type: "link"
url: string
title: string
children: Phrasing[]
style?: LinkStyle
}
type LinkStyle = 'onward'
Couldn'tonwardjust be arecommendation?
styleType?: 'onward-journey-link'
Can we omit the termtype? Juststylecould suffice
type LinkStyle = 'onward'
@adgad Could potential a LinkStyle separated type help to build a drop down menu for a Link component?
... define a style applied to the element based on the purpose of it.
@meel-io @adgad @apaleslimghost @elpopova The Content Tree is great, I'm a bit unsure about the principle behind the Tree and if we should bring some clarity. As mentioned in other PRs clarify perhaps some basic principles may help us build a better tree:
- What is the purpose of the tree? Is it a UI representation or is it a rich document?
E.g. Different meaningstyle: bluevsstyle: recommendation - (Not applicable to this PR) How strict do we want to be with types? A rich document may require more meaningful types that always strings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't onward just be a recommendation ?
I think this might get slightly confused with the recommended link type? @meel-io is "onward journey link" is how the terminology used by the stakeholders, design etc?
I would however suggest ditching the -link as it's kind of redundant here
Could potential a LinkStyle separated type help to build a drop down menu for a Link component?
Not something we do right now in Spark, but IMO having it as a separate type makes the purpose a bit clearer in the spec as well so I think it's a good idea!
What is the purpose of the tree? Is it a UI representation or is it a rich document?
I don't know if this directly answers the question, but IMO the tree should be capturing the data and editorial intent that drives the UI decisions, as opposed to the UI decisions themselves.
To take a concrete example, opinion toppers are all blue, but that itself isn't an active editorial decision - the decision is that it's an opinion topper, so in the tree we should only capture something like topperStyle=opinion. This would give us product flexibility to one day decide opinion = purple, without needing to republish all the content.
On the other hand, for split text toppers Editorial do explicitly choose a colour to match the image, so in the scenario the content tree should capture something like suggestedTopperColor=blue.
(we should deffo agree and document some of these principles!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is how the terminology used by the stakeholders
It would be good to align us with the stakeholders language. The challenges are about potentially duplications of similar terms because marketing people may us onward journey while editorial may recommend articles but they both mean a link too keep people engaged 😄 . onward-journey however seems fine to me here 👍
the tree should be capturing the data and editorial intent that drives the UI decisions
Yeah, I think we should doc the high level purpose of it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this might get slightly confused with the recommended link type? @meel-io is "onward journey link" is how the terminology used by the stakeholders, design etc?
I would however suggest ditching the -link as it's kind of redundant here
Yes, initially I thought I would map this value to something less redundant, like onward-journey on Content Pipeline but I decided against it to avoid confusion. But I will suggest changing it to onward-journey on C&M side.
|
I'll be happy, although it's still |
This attribute will be used to differentiate a link styled for partner content onward journey from standard links.
241d781 to
bd083dc
Compare
Sorry, somehow I forgot to push 🤦 |
Description
Anchor tags can include the
data-anchor-styleattribute to identify style variations from the standard link. It was introduced to differentiate the Partner Content onward journey link from the standard link.