-
Notifications
You must be signed in to change notification settings - Fork 173
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
wip: feat(linked logo section) - Build Linked Logo Section pattern #5464
base: main
Are you sure you want to change the base?
Conversation
Built the bones of this one ahead of the spec being fully finished in order to do some research on how the API might work before I submit the engineering side of the spec. Our usual method of building list-style HOCs (established here) is to create slots for each type of item content from {%- if slot == 'list_item_title_1' -%}
<h3 class="p-heading--5">Rich portfolio of products</h3>
{%- endif -%}
{%- if slot == 'list_item_description_1' -%}
<p>Resell the full range of Canonical’s portfolio of private and public
cloud products: OpenStack, Kubernetes, IoT, support, security and
compliance for Ubuntu.</p>
{%- endif -%}
{%- if slot == 'list_item_title_2' -%}
<h3 class="p-heading--5">Sales enablement</h3>
{%- endif -%}
{%- if slot == 'list_item_description_2' -%}
<p>Canonical can help train your field sales and presales teams and
provide you with sales collateral and permission to use the Canonical
and Ubuntu trademarks in your own materials.</p>
{%- endif -%}
{%- if slot == 'list_item_title_3' -%}
<h3 class="p-heading--5">Access to Canonical partner portal</h3>
{%- endif -%}
{%- if slot == 'list_item_description_3' -%}
<p>We host a partner portal with product and solutions collaterals, agreed
pricing, incentives and a deal registration system.</p>
{%- endif -%}
<!-- and so on...--> This does not trivially work for the linked logo section, because each item must be an I.E., consider this to be the full HTML of one of the items: <a href="#" aria-label="Kubeflow">
<div class="p-image-container--16-9 is-highlighted">
<img class="p-image-container__image" src="https://assets.ubuntu.com/v1/cd89477e-kubeflow-logo-container-vert-fill.png" alt="Kubeflow logo" width="432" height="481">
</div>
<hr class="p-rule--muted">
<p>Learn more ›</p>
</a> Under the old approach, the consuming developer would need to do one of the following:
{% if slot == "link_1" %}
href="#" aria-label="Kubeflow"
{% endif %}
{% if slot == "link_1_href" %}
#
{% endif %}
{% if slot == "link_1_label" %}
Kubeflow
{% endif %} Both of these feel a bit clumsy. So, I did some exploration of an alternate JSON-based approach to provide a bit more structure to the macro and improve the developer experience. The resulting consumer code looks like this: {{ vf_linked_logo_section(
title_text="PostgreSQL, security and support",
links=[
{
"href": "#",
"text": "Learn more ›",
"label": "Apache Spark",
"image_html": image(url="https://assets.ubuntu.com/v1/855deda8-apache-spark-logo-container-vert-fill.png",
alt="",
width="240",
height="481",
hi_def=True,
loading="auto",
attrs={"class": "p-image-container__image"}
) | safe
}
],
) }} |
9768c21
to
d1546d1
Compare
This makes me think if it would make sense to pass individual image attributes instead of It could look like this, with nested image props:
Or flattened:
|
There is definitely a value in being tool-agnostic. And this is the main drawback I see and I don't know what consequences it would have to require the There is probably less value in the field being free HTML. Because in such pre-defined pattern, do we want it to be free HTML? It seems we are expecting it to be an image, with a specific class name on top of it. This could benefit from being abstracted. But there is obviously a drawback of relying on There is a third option: to take image props directly and apply them in the pattern to |
Talked with @immortalcodes about this yesterday. He raised similar points as @bartaz but we arrived at raw HTML for the image as being the best approach, as sites generally uses the image template this must be compatible with the image template & cloudinary in order to create the least friction in their work. We should also avoid creating downstream dependencies on the image template. The usage of the raw HTML should be adequately documented such that its usage (and expectation of In the future, when we are using React-based HOCs, this will become much simpler as we will be able to take advantage of more robust input handling and controls. This approach is a good medium-term solution. |
Done
Builds the linked logo section pattern/HOC
Fixes WD-17120
QA
Check if PR is ready for release
If this PR contains Vanilla SCSS or macro code changes, it should contain the following changes to make sure it's ready for the release:
Feature 🎁
,Breaking Change 💣
,Bug 🐛
,Documentation 📝
,Maintenance 🔨
.package.json
should be updated relative to the most recent release, following semver conventionScreenshots