File Upload disabled clean-up
#5719
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
patrickpatrickpatrick
changed the base branch from
main
to
spike-enhanced-file-upload
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-5719
📋 StatsFile sizes
Modules
View stats and visualisations on the review app Action run for 7e06c98 |
JavaScript changes to npm packagediff --git a/packages/govuk-frontend/dist/govuk/govuk-frontend.min.js b/packages/govuk-frontend/dist/govuk/govuk-frontend.min.js
index 17354224d..f6220464f 100644
--- a/packages/govuk-frontend/dist/govuk/govuk-frontend.min.js
+++ b/packages/govuk-frontend/dist/govuk/govuk-frontend.min.js
@@ -782,11 +782,11 @@ class FileUpload extends ConfigurableComponent {
})), this.$root.insertAdjacentElement("afterbegin", s), this.$input.setAttribute("tabindex", "-1"), this.$input.setAttribute("aria-hidden", "true"), this.$button = s, this.$status = r, this.$input.addEventListener("change", this.onChange.bind(this)), this.updateDisabledState(), this.observeDisabledState(), this.$announcements = document.createElement("span"), this.$announcements.classList.add("govuk-file-upload-announcements"), this.$announcements.classList.add("govuk-visually-hidden"), this.$announcements.setAttribute("aria-live", "assertive"), this.$root.insertAdjacentElement("afterend", this.$announcements), this.$button.addEventListener("drop", this.onDrop.bind(this)), document.addEventListener("dragenter", this.updateDropzoneVisibility.bind(this)), document.addEventListener("dragenter", (() => {
this.enteredAnotherElement = !0
})), document.addEventListener("dragleave", (() => {
- this.enteredAnotherElement || (this.hideDraggingState(), this.$announcements.innerText = this.i18n.t("leftDropZone")), this.enteredAnotherElement = !1
+ this.enteredAnotherElement || this.$button.disabled || (this.hideDraggingState(), this.$announcements.innerText = this.i18n.t("leftDropZone")), this.enteredAnotherElement = !1
}))
}
updateDropzoneVisibility(t) {
- t.target instanceof Node && (this.$root.contains(t.target) ? t.dataTransfer && isContainingFiles(t.dataTransfer) && (this.$button.classList.contains("govuk-file-upload-button--dragging") || (this.showDraggingState(), this.$announcements.innerText = this.i18n.t("enteredDropZone"))) : this.$button.classList.contains("govuk-file-upload-button--dragging") && (this.hideDraggingState(), this.$announcements.innerText = this.i18n.t("leftDropZone")))
+ this.$button.disabled || t.target instanceof Node && (this.$root.contains(t.target) ? t.dataTransfer && isContainingFiles(t.dataTransfer) && (this.$button.classList.contains("govuk-file-upload-button--dragging") || (this.showDraggingState(), this.$announcements.innerText = this.i18n.t("enteredDropZone"))) : this.$button.classList.contains("govuk-file-upload-button--dragging") && (this.hideDraggingState(), this.$announcements.innerText = this.i18n.t("leftDropZone")))
}
showDraggingState() {
this.$button.classList.add("govuk-file-upload-button--dragging")
Action run for 7e06c98 |
Stylesheets changes to npm packagediff --git a/packages/govuk-frontend/dist/govuk/govuk-frontend.min.css b/packages/govuk-frontend/dist/govuk/govuk-frontend.min.css
index 27286ad40..5bef099c0 100644
--- a/packages/govuk-frontend/dist/govuk/govuk-frontend.min.css
+++ b/packages/govuk-frontend/dist/govuk/govuk-frontend.min.css
@@ -3511,6 +3511,11 @@ screen and (forced-colors:active) {
background-color: #d2e2f1
}
+.govuk-file-upload-button--dragging {
+ border-style: solid;
+ border-color: #0b0c0c
+}
+
.govuk-file-upload-button--dragging.govuk-file-upload-button {
background-color: #c1c3c5
}
@@ -3519,19 +3524,15 @@ screen and (forced-colors:active) {
background-color: #f3f2f1
}
-.govuk-file-upload-button--dragging:not(:disabled) {
- border-color: #8e9092
-}
-
-.govuk-file-upload-button--dragging:not(:disabled) .govuk-file-upload-button__pseudo-button {
- background-color: #dbdad9
-}
-
.govuk-file-upload-button--dragging.govuk-file-upload-button--empty .govuk-file-upload-button__pseudo-button,
.govuk-file-upload-button--dragging.govuk-file-upload-button--empty:not(:disabled) .govuk-file-upload-button__status {
background-color: #fff
}
+.govuk-file-upload-button--dragging .govuk-file-upload-button__pseudo-button {
+ background-color: #dbdad9
+}
+
.govuk-footer {
font-family: GDS Transport, arial, sans-serif;
-webkit-font-smoothing: antialiased;
Action run for 7e06c98 |
Other changes to npm packagediff --git a/packages/govuk-frontend/dist/govuk/all.bundle.js b/packages/govuk-frontend/dist/govuk/all.bundle.js
index 54140f7f8..b57e1a9a6 100644
--- a/packages/govuk-frontend/dist/govuk/all.bundle.js
+++ b/packages/govuk-frontend/dist/govuk/all.bundle.js
@@ -1756,7 +1756,7 @@
this.enteredAnotherElement = true;
});
document.addEventListener('dragleave', () => {
- if (!this.enteredAnotherElement) {
+ if (!this.enteredAnotherElement && !this.$button.disabled) {
this.hideDraggingState();
this.$announcements.innerText = this.i18n.t('leftDropZone');
}
@@ -1770,6 +1770,7 @@
* @param {DragEvent} event - The `dragenter` event
*/
updateDropzoneVisibility(event) {
+ if (this.$button.disabled) return;
if (event.target instanceof Node) {
if (this.$root.contains(event.target)) {
if (event.dataTransfer && isContainingFiles(event.dataTransfer)) {
diff --git a/packages/govuk-frontend/dist/govuk/all.bundle.mjs b/packages/govuk-frontend/dist/govuk/all.bundle.mjs
index ea5e2d5bd..bd59d9bb4 100644
--- a/packages/govuk-frontend/dist/govuk/all.bundle.mjs
+++ b/packages/govuk-frontend/dist/govuk/all.bundle.mjs
@@ -1750,7 +1750,7 @@ class FileUpload extends ConfigurableComponent {
this.enteredAnotherElement = true;
});
document.addEventListener('dragleave', () => {
- if (!this.enteredAnotherElement) {
+ if (!this.enteredAnotherElement && !this.$button.disabled) {
this.hideDraggingState();
this.$announcements.innerText = this.i18n.t('leftDropZone');
}
@@ -1764,6 +1764,7 @@ class FileUpload extends ConfigurableComponent {
* @param {DragEvent} event - The `dragenter` event
*/
updateDropzoneVisibility(event) {
+ if (this.$button.disabled) return;
if (event.target instanceof Node) {
if (this.$root.contains(event.target)) {
if (event.dataTransfer && isContainingFiles(event.dataTransfer)) {
diff --git a/packages/govuk-frontend/dist/govuk/components/file-upload/_index.scss b/packages/govuk-frontend/dist/govuk/components/file-upload/_index.scss
index bc63b2e21..08c9705e5 100644
--- a/packages/govuk-frontend/dist/govuk/components/file-upload/_index.scss
+++ b/packages/govuk-frontend/dist/govuk/components/file-upload/_index.scss
@@ -178,6 +178,9 @@
}
.govuk-file-upload-button--dragging {
+ border-style: solid;
+ border-color: govuk-colour("black");
+
// extra specificity to apply when
// empty
&.govuk-file-upload-button {
@@ -188,18 +191,14 @@
background-color: govuk-colour("light-grey");
}
- &:not(:disabled) {
- border-color: govuk-shade(govuk-colour("mid-grey"), 20%);
-
- .govuk-file-upload-button__pseudo-button {
- background-color: govuk-shade(govuk-colour("light-grey"), 10%);
- }
- }
-
&.govuk-file-upload-button--empty:not(:disabled) .govuk-file-upload-button__status,
&.govuk-file-upload-button--empty .govuk-file-upload-button__pseudo-button {
background-color: govuk-colour("white");
}
+
+ .govuk-file-upload-button__pseudo-button {
+ background-color: govuk-shade(govuk-colour("light-grey"), 10%);
+ }
}
}
diff --git a/packages/govuk-frontend/dist/govuk/components/file-upload/file-upload.bundle.js b/packages/govuk-frontend/dist/govuk/components/file-upload/file-upload.bundle.js
index fcb686c94..6c37a8243 100644
--- a/packages/govuk-frontend/dist/govuk/components/file-upload/file-upload.bundle.js
+++ b/packages/govuk-frontend/dist/govuk/components/file-upload/file-upload.bundle.js
@@ -583,7 +583,7 @@
this.enteredAnotherElement = true;
});
document.addEventListener('dragleave', () => {
- if (!this.enteredAnotherElement) {
+ if (!this.enteredAnotherElement && !this.$button.disabled) {
this.hideDraggingState();
this.$announcements.innerText = this.i18n.t('leftDropZone');
}
@@ -597,6 +597,7 @@
* @param {DragEvent} event - The `dragenter` event
*/
updateDropzoneVisibility(event) {
+ if (this.$button.disabled) return;
if (event.target instanceof Node) {
if (this.$root.contains(event.target)) {
if (event.dataTransfer && isContainingFiles(event.dataTransfer)) {
diff --git a/packages/govuk-frontend/dist/govuk/components/file-upload/file-upload.bundle.mjs b/packages/govuk-frontend/dist/govuk/components/file-upload/file-upload.bundle.mjs
index 102b65489..307cbbd67 100644
--- a/packages/govuk-frontend/dist/govuk/components/file-upload/file-upload.bundle.mjs
+++ b/packages/govuk-frontend/dist/govuk/components/file-upload/file-upload.bundle.mjs
@@ -577,7 +577,7 @@ class FileUpload extends ConfigurableComponent {
this.enteredAnotherElement = true;
});
document.addEventListener('dragleave', () => {
- if (!this.enteredAnotherElement) {
+ if (!this.enteredAnotherElement && !this.$button.disabled) {
this.hideDraggingState();
this.$announcements.innerText = this.i18n.t('leftDropZone');
}
@@ -591,6 +591,7 @@ class FileUpload extends ConfigurableComponent {
* @param {DragEvent} event - The `dragenter` event
*/
updateDropzoneVisibility(event) {
+ if (this.$button.disabled) return;
if (event.target instanceof Node) {
if (this.$root.contains(event.target)) {
if (event.dataTransfer && isContainingFiles(event.dataTransfer)) {
diff --git a/packages/govuk-frontend/dist/govuk/components/file-upload/file-upload.mjs b/packages/govuk-frontend/dist/govuk/components/file-upload/file-upload.mjs
index c1a28be19..482b1798c 100644
--- a/packages/govuk-frontend/dist/govuk/components/file-upload/file-upload.mjs
+++ b/packages/govuk-frontend/dist/govuk/components/file-upload/file-upload.mjs
@@ -98,7 +98,7 @@ class FileUpload extends ConfigurableComponent {
this.enteredAnotherElement = true;
});
document.addEventListener('dragleave', () => {
- if (!this.enteredAnotherElement) {
+ if (!this.enteredAnotherElement && !this.$button.disabled) {
this.hideDraggingState();
this.$announcements.innerText = this.i18n.t('leftDropZone');
}
@@ -112,6 +112,7 @@ class FileUpload extends ConfigurableComponent {
* @param {DragEvent} event - The `dragenter` event
*/
updateDropzoneVisibility(event) {
+ if (this.$button.disabled) return;
if (event.target instanceof Node) {
if (this.$root.contains(event.target)) {
if (event.dataTransfer && isContainingFiles(event.dataTransfer)) {
Action run for 7e06c98 |
There was a problem hiding this comment.
Noticed a little issue with the styles changine when dragging over a disabled component, which led to further thinking about the disabled state. If we confirm that the state shouldn't change when disabled, we can expand this PR to do both the black border and the tweaks to disabled styles and screen reader announcements.
| @@ -178,6 +178,8 @@ | |||
| } | |||
|
|
|||
| .govuk-file-upload-button--dragging { | |||
| border-style: solid; | |||
There was a problem hiding this comment.
issue This attracted my attention to the fact that the styles change when dragging over a disabled button, while the component won't react to a file being dropped.
@CharlotteDowns thinking it would make sense, when the button is disabled, for it not to react at all visually. Any objection?
@patrickpatrickpatrick if confirmed that we don't want the button to react, it might be easier to check in the JavaScript for the button's disabled property (not attribute, as the button mayb be disabled because it's inside a disabled <fieldset>) before adding .govuk-file-upload-button--dragging, which would avoid us the :not(:disabled) selector. This would also allow us to prevent screen reader announcements as well when the component is disabled.
There was a problem hiding this comment.
No objection about it not reacting. Once we've made this change are we ready to release?
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-5719
patrickpatrickpatrick
changed the title
patrickpatrickpatrick
changed the title
disabled clean-up
patrickpatrickpatrick
force-pushed
the
add-back-border
branch
from
411b4ce to
7e06c98
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-5719
romaricpascal
approved these changes
Feb 13, 2025
There was a problem hiding this comment.
Dragging styles are fixed, thanks @patrickpatrickpatrick ! @CharlotteDowns I'll make a round on the issue for what's left to do.
Comment on lines
+353
to
+375
| it('does not appear if button disabled', async () => { | ||
| await render(page, 'file-upload', examples.enhanced, { | ||
| beforeInitialisation() { | ||
| document | ||
| .querySelector('[type="file"]') | ||
| .setAttribute('disabled', '') | ||
| } | ||
| }) | ||
|
|
||
| await page.mouse.dragEnter( | ||
| { x: wrapperBoundingBox.x + 1, y: wrapperBoundingBox.y + 1 }, | ||
| structuredClone(dragData) | ||
| ) | ||
|
|
||
| const disabledAnnouncement = await page.$( | ||
| '.govuk-file-upload-announcements' | ||
| ) | ||
|
|
||
| await expect(page.$(selectorDropzoneHidden)).resolves.toBeTruthy() | ||
| await expect( | ||
| disabledAnnouncement.evaluate((e) => e.textContent) | ||
| ).resolves.toBe('') | ||
| }) |
patrickpatrickpatrick
merged commit 85e94b6
into
spike-enhanced-file-upload
46 checks passed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
--draggingto the the File Upload button if it is disabledLeft drop zoneif the File Upload button is disabledWhy
Black border when drag zone active had disappeared, possibly from refactoring the CSS in prior commit.
Refactoring had resulted in the dropzone not behaving as expected when button was disabled.