-
-
Notifications
You must be signed in to change notification settings - Fork 944
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
SAK-40229 Content screen reader announcing file upload to dropbox #12995
Conversation
content/content-tool/tool/src/webapp/vm/resources/sakai_dropbox_multiple_folders_upload.vm
Outdated
Show resolved
Hide resolved
content/content-tool/tool/src/webapp/vm/resources/sakai_dropbox_multiple_folders_upload.vm
Outdated
Show resolved
Hide resolved
content/content-tool/tool/src/webapp/vm/resources/sakai_dropbox_multiple_folders_upload.vm
Outdated
Show resolved
Hide resolved
content/content-tool/tool/src/webapp/vm/resources/sakai_dropbox_multiple_folders_upload.vm
Outdated
Show resolved
Hide resolved
else | ||
{ | ||
feedback.classList.add('d-none'); | ||
nameField.value = ''; | ||
} |
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.
Do we need else
block ? when there's no file selected nothing changes and d-none
is already present.
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 checked that when I deselect a file, the message and display name do not disappear. That's why I added the else block and it works as intended.
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.
@kunaljaykam can you please review this and let me know if any changes are required?
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.
You are correct. I tested your pr. The else
block is necessary to clear outdated file info when the file picker is opened but no new file is selected. Without it, the old file name and feedback message remain and causes console error.
I think we should re-write the whole handleFileChange
function it looks messy, and include the display name field inside; fileField.files.length > 0) check as well so display name gets updates as well.
content/content-tool/tool/src/webapp/vm/resources/sakai_dropbox_multiple_folders_upload.vm
Outdated
Show resolved
Hide resolved
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.
Looks good to me. Thanks @Sharadhi98
nameField.value = fileField.files[0].name; | ||
} | ||
previousFileName = fileField.files[0].name; | ||
## Goal: update the display name field. But if the user provided their own display name, don't overwrite 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.
spacing
No description provided.