Skip to content
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.

Sign up for GitHub

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

feat(file): Added download attributes. - OEL-2772 #573

Merged
merged 7 commits into from
Aug 1, 2024
Merged

feat(file): Added download attributes. - OEL-2772 #573

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
43d7c4f
OEL-2772: Added attributes on download button - file.
tibi2303 Jan 12, 2024
b47fce8
Merge branch 'development' into OEL-2772-#default#
tibi2303 Mar 6, 2024
a1a3aa8
Merge branch 'development' into OEL-2772-#default#
tibi2303 Jul 26, 2024
35f0396
OEL-2772: Custom download icon.
tibi2303 Jul 26, 2024
e8f9d1c
OEL-2772: Custom download icon.
tibi2303 Jul 26, 2024
04c42c7
OEL-2772: PR feedback.
tibi2303 Jul 31, 2024
da29705
OEL-2772: Changed approach of attributes.
tibi2303 Aug 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
286 changes: 286 additions & 0 deletions src/compositions/bcl-file/__snapshots__/file.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,292 @@ exports[`OE - File Card renders correctly 1`] = `
</jest>
`;

exports[`OE - File Card renders correctly with different download icon 1`] = `
<jest>
<div
class="bcl-file border rounded"
>
<div
class="border-bottom px-3-5 py-3 d-md-flex"
>
<div
class="pe-2 pb-2"
>
<svg
class="mt-1 icon--file bi icon--xl"
>
<use
xlink:href="/icons.svg#file-pdf-fill"
/>
</svg>
</div>
<div>
<p
class="fw-medium mb-2 fs-5"
>
Document title
</p>
<p
class="mb-3"
>
Lorem ipsum dolor sit amet, consectetur adipiscing elit.
</p>
<span
class="text-muted d-inline-block me-3 mb-2 mb-md-0 text-nowrap"
>
Article
</span>
<span
class="text-muted d-inline-block me-3 text-nowrap"
>
Published : 3 September 2022
</span>
</div>
</div>
<div
class="px-3-5 py-3"
>
<div
class="d-md-flex"
>
<div
class="d-md-flex flex-grow-1"
>
<div
class="flex-grow-1"
>
<small
class="fw-medium m-0"
>
English
<span
class="fw-normal"
>
(16.2 MB - PDF)
</span>
</small>
</div>
<a
class="standalone align-self-center d-inline-block mt-1 mt-md-0 flex-shrink-0"
download="true"
href="/example.html"
target="_blank"
>
Download
<svg
class="ms-2-5 bi icon--fluid"
>
<use
xlink:href="#cloud-download"
/>
</svg>
</a>
</div>
</div>
</div>
<div
class="bg-lighter py-3 px-4 border-top"
>
<div
class="text-end text-md-center"
>
<a
aria-controls="language-dropdown"
aria-expanded="false"
class="standalone"
data-bs-toggle="collapse"
href="#language-dropdown"
role="button"
>
Other languages (3)
<svg
class="ms-2-5 bi icon--2xs"
>
<use
xlink:href="/icons.svg#caret-down-fill"
/>
</svg>
</a>
</div>
<div
class="collapse"
id="language-dropdown"
>
<div
class="py-3 border-bottom"
>
<div
class="row"
>
<div
class="col-12 col-md-9"
>
<p
class="fw-bold m-0"
>
File title FR
</p>
<p
class="m-0"
>
Lorem ipsum dolor sit amet, consectetur adipiscing elit.
</p>
<small
class="fw-bold m-0"
>
Français
<span
class="fw-normal"
>
(16.2 MB - PDF)
</span>
</small>
</div>
<div
class="col-12 col-md-3 align-self-end text-md-end"
>
<a
class="d-block standalone mt-1"
download="true"
href="/example.html"
target="_blank"
>
Download
<svg
class="ms-2-5 bi icon--fluid"
>
<use
xlink:href="/icons.svg#download"
/>
</svg>
</a>
</div>
</div>
</div>
<div
class="py-3 border-bottom"
>
<div
class="row"
>
<div
class="col-12 col-md-9"
>
<p
class="fw-bold m-0"
>
File title ES
</p>
<p
class="m-0"
>
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Duis fermentum facilisis ex, ac porta massa. Lorem ipsum dolor sit amet, consectetur adipiscing elit.
<br />
Duis fermentum facilisis ex, ac porta massa.
</p>
<small
class="fw-bold m-0"
>
Español
<span
class="fw-normal"
>
(16.2 MB - PDF)
</span>
</small>
</div>
<div
class="col-12 col-md-3 align-self-end text-md-end"
>
<a
class="d-block standalone mt-1"
download="true"
href="/example.html"
target="_blank"
>
Download
<svg
class="ms-2-5 bi icon--fluid"
>
<use
xlink:href="/icons.svg#download"
/>
</svg>
</a>
</div>
</div>
</div>
<div
class="py-3 border-bottom"
>
<div
class="row"
>
<div
class="col-12 col-md-9"
>
<p
class="fw-bold m-0"
>
File title BG
</p>
<p
class="m-0"
>
Lorem ipsum dolor sit amet, consectetur adipiscing elit.
</p>
<small
class="fw-bold m-0"
>
български
<span
class="fw-normal"
>
(16.2 MB - PDF)
</span>
</small>
</div>
<div
class="col-12 col-md-3 align-self-end text-md-end"
>
<a
class="d-block standalone mt-1"
download="true"
href="/example.html"
target="_blank"
>
Download
<svg
class="ms-2-5 bi icon--fluid"
>
<use
xlink:href="/icons.svg#download"
/>
</svg>
</a>
</div>
</div>
</div>
<div
class="pt-3"
>
<p
class="mb-2 d-inline-block me-2"
>
Looking for another language which is not on the list?
</p>
<a
href="/example.html"
>
Find out why.
</a>
</div>
</div>
</div>
</div>
</jest>
`;

exports[`OE - File Card renders correctly with title 1`] = `
<jest>
<h6
Expand Down
11 changes: 7 additions & 4 deletions src/compositions/bcl-file/file.html.twig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,13 +139,16 @@
{% endif %}
</div>
{% set _download = _download|merge({
icon_position: "after",
icon: {
icon: _download.icon|default({}|merge({
name: "download",
size: "fluid",
path: _icon_path,
},
attributes: create_attribute()
})),
attributes: _download.attributes|default(create_attribute())
}) %}
{% set _download = _download|merge({
icon_position: "after",
attributes: _download.attributes
.setAttribute('download', true)
.setAttribute('target', '_blank')
.addClass(['standalone', 'align-self-center', 'd-inline-block', 'mt-1', 'mt-md-0', 'flex-shrink-0'])
Copy link

Choose a reason for hiding this comment

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

Suggested change
attributes: _download.attributes
.setAttribute('download', true)
.setAttribute('target', '_blank')
.addClass(['standalone', 'align-self-center', 'd-inline-block', 'mt-1', 'mt-md-0', 'flex-shrink-0'])
attributes: _download.attributes
.addClass(['standalone', 'align-self-center', 'd-inline-block', 'mt-1', 'mt-md-0', 'flex-shrink-0'])

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It was failing like this, it's more or less how you suggested now but without the attributes merge.

Copy link

Choose a reason for hiding this comment

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

Hello misters,

Why not keeping my original logic #583 ?

  1. Set the BCL default attributes
  2. Merge with possible provided child that can eventually remove/update/add attributes

For example, in your case you are forcing the download=true, I do not need it sometimes
See : https://historia.europa.eu/en/about-us/press (icon changed + attributes changed)

Expand Down
17 changes: 17 additions & 0 deletions src/compositions/bcl-file/file.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,23 @@ describe("OE - File Card", () => {
).resolves.toMatchSnapshot();
});

test(`renders correctly with different download icon`, () => {
expect.assertions(1);
return expect(
render({
...demoCardData,
download: {
label: "Download",
path: "/example.html",
icon: {
name: "cloud-download",
size: "fluid",
Copy link

Choose a reason for hiding this comment

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

I wish we could test this with a different icon set :) But it's ok.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In the test you can see that it's not populated... I could add but we have only that sprite :)) and i think it's enough that it's not populated if you dont add it (that means you can change it how you want).

},
},
})
).resolves.toMatchSnapshot();
});

test(`passes the accessibility tests`, async () => {
expect(
await axe(renderTwigFileAsHtml(template, demoCardData, true))
Expand Down
Loading