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.

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

FormElement: Add FileElement #66

Merged
merged 4 commits into from Jan 24, 2023
Merged

FormElement: Add FileElement #66

merged 4 commits into from Jan 24, 2023

Conversation

sukhwinder33445
Copy link
Contributor

@sukhwinder33445 sukhwinder33445 commented Jul 15, 2022

Blocked by

@cla-bot cla-bot bot added the cla/signed label Jul 15, 2022
@sukhwinder33445 sukhwinder33445 self-assigned this Jul 22, 2022
@sukhwinder33445 sukhwinder33445 force-pushed the add-file-element branch 2 times, most recently from 9d59894 to c749ba7 Compare July 22, 2022 09:32
@sukhwinder33445 sukhwinder33445 force-pushed the add-file-element branch 2 times, most recently from 1d55608 to f26e1db Compare August 5, 2022 12:56
@sukhwinder33445 sukhwinder33445 force-pushed the add-file-element branch 2 times, most recently from cb3b085 to 00d271f Compare September 8, 2022 12:26
@sukhwinder33445
Copy link
Contributor Author

@cla-bot check

@sukhwinder33445 sukhwinder33445 added the enhancement New feature or request label Oct 5, 2022
Copy link
Member

@lippserd lippserd left a comment

Choose a reason for hiding this comment

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

In addition to the changes requested, please test actual requests with ServerRequestInterface:: withUploadedFiles().

src/FormElement/FileElement.php Outdated Show resolved Hide resolved
src/FormElement/FileElement.php Outdated Show resolved Hide resolved
src/FormElement/FileElement.php Outdated Show resolved Hide resolved
src/FormElement/FileElement.php Outdated Show resolved Hide resolved
src/FormElement/FileElement.php Outdated Show resolved Hide resolved
tests/FormElement/FileElementTest.php Outdated Show resolved Hide resolved
tests/FormElement/FileElementTest.php Outdated Show resolved Hide resolved
tests/FormElement/FileElementTest.php Outdated Show resolved Hide resolved
tests/FormElement/FileElementTest.php Show resolved Hide resolved
tests/FormElement/FileElementTest.php Outdated Show resolved Hide resolved
Copy link
Member

@nilmerg nilmerg left a comment

Choose a reason for hiding this comment

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

While looking at the testAcceptAttribute test I wonder if it makes sense to automatically add the FileValidator. Should be considered, IMHO.

tests/FormElement/FileElementTest.php Outdated Show resolved Hide resolved
src/FormElement/FileElement.php Show resolved Hide resolved
tests/FormElement/FileElementTest.php Outdated Show resolved Hide resolved
@sukhwinder33445 sukhwinder33445 force-pushed the add-file-element branch 2 times, most recently from 6d850ad to 80bb328 Compare November 17, 2022 11:31
@sukhwinder33445 sukhwinder33445 changed the base branch from master to enhance-select-element November 17, 2022 11:33
@sukhwinder33445 sukhwinder33445 force-pushed the add-file-element branch 2 times, most recently from 2965105 to 2b57da1 Compare November 18, 2022 15:37
@sukhwinder33445 sukhwinder33445 force-pushed the enhance-select-element branch 2 times, most recently from f34c721 to c0bf9cf Compare November 28, 2022 11:27
@sukhwinder33445 sukhwinder33445 force-pushed the enhance-select-element branch 12 times, most recently from 7efb211 to dcd3d49 Compare November 30, 2022 14:17
@sukhwinder33445 sukhwinder33445 force-pushed the enhance-select-element branch 2 times, most recently from 10e6fea to b5de880 Compare December 21, 2022 15:58
Base automatically changed from enhance-select-element to master January 10, 2023 08:38
@nilmerg nilmerg merged commit c2bda62 into master Jan 24, 2023
@nilmerg nilmerg deleted the add-file-element branch January 24, 2023 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla/signed enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants