Skip to content

fix(b-form-file): ensure drag/drop handles required prop correctly (except IE11), add drag/drop accept type handling, fix drop in IE11 (closes #3673) #3674

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

Closed
wants to merge 171 commits into from

Conversation

tmorehouse
Copy link
Member

@tmorehouse tmorehouse commented Jul 11, 2019

Describe the PR

Note: New PR required

Closes #3673 (except for IE 11, sorry)

Drag and drop files bypasses the file input altogether, which makes native browser validation constraints (i.e. required) not work, as the input does not reflect the dropped files.

Chrome, Firefox, Safari, Edge support dropping files directly on a file input, but IE11 and some older browser versions do not, so we can't take advantage of the native file input drop features (which would simplify the code drastically, and retain proper validity state).

All modern browsers support setting InputElement.files to a FileList array (which will satisfy required constraint), but again IE 11 doesn't support this (could be placed in a try/catch to trap/mask errors).

What's new and changes:

  • Drag and drop now attempts to set the files array on the native file input (flattened FileList, filtered by accept criteria), so that required prop works with drag/drop (except on IE, the attempt to set the files is wrapped in a try/catch to suppress the error)
  • Drag/drop now respects the accept criteria. Converts the accept prop to an array of regular expressions for testing the file type/extension of dropped files
  • Drag and drop did now respects the directory setting (Except IE, which doesn't support directory mode)
  • Uses a negative z-index on the opaque file input so that it doesn't interfere with drag/drop on IE (and other browsers). Clicking the custom input label will still open the native file browser (as the label is associated with the input via the for attribute).
  • Added class overflow-hidden to custom label to prevent filenames from "breaking out" of custom file control
  • filename array and files array that is passed to scoped slot are now always flattened (even in directory mode), but includes a new scoped property which provides the files in traversed mode
  • Also adds a few fixes to the form-file docs

TODO:

  • accept file matching tests
  • Get drag/drop to work correctly on IE 11
  • Set dropEffect to 'none' when file(s) can't be dropped (works on all but IE 11, as files are not known until the drop event happens)
  • Add optional prop/slot for prompt when file's can't be dropped - or make drop-placeholder slot scoped with a valid scope prop (not applicable to IE 11, as files aren't known until dropped)
  • Add notes in docs about limitations of required prop with drag/drop on IE11
  • document requirement of Promise support for drag and drop mode
  • Add slots for placeholder prompt, and drop-placeholder prompt, as an alternative to the props.
  • document new slots
  • add new slots to package.json meta
  • minor custom SCSS updates to handle overflowing selected file name(s)
  • additional test suites

Notes: Bootstrap v5 has a new structure for the custom label, which allows for better flexibility in handling overflowing names. We may want to consider using this newer format with Bootstrap v4.x (with a bit of custom SCSS, with variable sniffing) to take advantage of the new structure (and prepare for v5 when it is released)

PR checklist

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Enhancement
  • ARIA accessibility
  • Documentation update
  • Other (please describe)

Does this PR introduce a breaking change? (check one)

  • No
  • Yes (please describe)

The PR fulfills these requirements:

  • It's submitted to the dev branch, not the master branch
  • When resolving a specific issue, it's referenced in the PR's title (i.e. [...] (fixes #xxx[,#xxx]), where "xxx" is the issue number)
  • It should address only one issue or feature. If adding multiple features or fixing a bug and adding a new feature, break them into separate PRs if at all possible.
  • The title should follow the Conventional Commits naming convention (i.e. fix(alert): not alerting during SSR render, docs(badge): update pill examples, fix typos, chore: fix typo in README, etc). This is very important, as the CHANGELOG is generated from these messages.

If new features/enhancement/fixes are added or changed:

  • Includes documentation updates (including updating the component's package.json for slot and event changes)
  • New/updated tests are included and passing (if required)
  • Existing test suites are passing
  • The changes have not impacted the functionality of other components or directives
  • ARIA Accessibility has been taken into consideration (Does it affect screen reader users or keyboard only users? Clickable items should be in the tab index, etc.)

If adding a new feature, or changing the functionality of an existing feature, the PR's
description above includes:

  • A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

@codecov
Copy link

codecov bot commented Jul 11, 2019

Codecov Report

Merging #3674 into dev will decrease coverage by 0.19%.
The diff coverage is 87.64%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #3674      +/-   ##
==========================================
- Coverage   99.86%   99.66%   -0.20%     
==========================================
  Files         238      236       -2     
  Lines        4470     4519      +49     
  Branches     1260     1273      +13     
==========================================
+ Hits         4464     4504      +40     
- Misses          5       11       +6     
- Partials        1        4       +3     
Impacted Files Coverage Δ
src/components/breadcrumb/breadcrumb.js 100.00% <ø> (ø)
src/components/form-group/form-group.js 100.00% <ø> (ø)
src/components/pagination-nav/pagination-nav.js 100.00% <ø> (ø)
src/components/table/helpers/mixin-tbody-row.js 100.00% <ø> (ø)
src/components/table/helpers/table-cell.js 100.00% <ø> (ø)
src/mixins/pagination.js 100.00% <ø> (ø)
src/utils/prefix-prop-name.js 100.00% <ø> (ø)
src/utils/router.js 100.00% <ø> (ø)
src/utils/suffix-prop-name.js 100.00% <ø> (ø)
src/utils/unprefix-prop-name.js 100.00% <ø> (ø)
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2cf6d25...d2b8569. Read the comment docs.

@tmorehouse tmorehouse marked this pull request as ready for review July 11, 2019 14:46
@tmorehouse tmorehouse requested a review from jacobmllr95 July 11, 2019 14:46
@tmorehouse tmorehouse changed the title fix(b-form-file): ensure drag/drop handles required prop correctly, add drag/drop accept type handling, fix drop in IE11 (closes #3673) fix(b-form-file): ensure drag/drop handles required prop correctly (except IE11), add drag/drop accept type handling, fix drop in IE11 (closes #3673) Aug 13, 2019
@tmorehouse tmorehouse added PR: Minor Requires minor version bump and removed PR: Patch Requires patch version bump labels Sep 19, 2019
@jacobmllr95 jacobmllr95 closed this Aug 4, 2020
@jacobmllr95 jacobmllr95 deleted the issue/3673 branch September 10, 2020 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dragged file input still shows required alert
2 participants