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

Jump to bottom

feat: introduce File Uploader component #2947

Merged
merged 18 commits into from
Aug 18, 2020
Merged

feat: introduce File Uploader component #2947

merged 18 commits into from
Aug 18, 2020

Conversation

manu-kr
Copy link
Contributor

@manu-kr manu-kr commented Jul 31, 2020 •
edited by InnaAtanasova

Please provide a link to the associated issue.

Closes: #2927
Blocked by: #2882

Please provide a brief summary of this pull request.

Creating new 'File Uploader' Component.

Please check whether the PR fulfills the following requirements

@manu-kr manu-kr self-assigned this Jul 31, 2020
@manu-kr manu-kr added this to the Sprint 42 - Santorini milestone Jul 31, 2020
@netlify
Copy link

netlify bot commented Jul 31, 2020 •
edited

❌ Check the deploy log for errors here: https://app.netlify.com/sites/fundamental-ngx/deploys

🔨 Explore the source changes: 464e5ec

🔍 Inspect the deploy logs: https://app.netlify.com/sites/fundamental-ngx/deploys/5f3a21b711f1950007a17a95

kavya-b
kavya-b reviewed Jul 31, 2020
View reviewed changes
libs/core/src/lib/file-uploader/directives/file-uploader-dragndrop.directive.ts Outdated
export class FileUploaderDragndropDirective {
/** Whether multiple files can be dropped at once. */
@Input()
multiple: boolean = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

have you run linter? Seems like strict typing enforcement has been removed recently so types may not be needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes fixed all lint errors

kavya-b
kavya-b reviewed Jul 31, 2020
View reviewed changes
libs/core/src/lib/file-uploader/file-uploader.component.ts Outdated

fileName: string = '';

valid_files: File[] = [];
Copy link
Contributor

Choose a reason for hiding this comment

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

missing comments, again not sure of inconsistency in naming, camel cases vs underscores

Copy link
Member

Choose a reason for hiding this comment

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

Please use camel-case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

used camelCase

kavya-b
kavya-b reviewed Jul 31, 2020
View reviewed changes
package.json Show resolved Hide resolved
@salarenko salarenko added blocked blocked ticket core Core library specific issues labels Jul 31, 2020
KevinOkamoto
KevinOkamoto suggested changes Jul 31, 2020
View reviewed changes
Copy link
Member

@KevinOkamoto KevinOkamoto left a comment

Choose a reason for hiding this comment

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

Please add unit tests for parseFileSize

libs/core/src/lib/file-uploader/file-uploader.component.ts Outdated

fileName: string = '';

valid_files: File[] = [];
Copy link
Member

Choose a reason for hiding this comment

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

Please use camel-case.

libs/core/src/lib/file-uploader/file-uploader.component.ts Outdated
this.inputRefText.nativeElement.title = fileName;
}

parseFileSize(filesize: string) {
Copy link
Member

Choose a reason for hiding this comment

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

Can you add unit tests to the spec for this function which test all the valid inputs and a few invalid inputs as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Written unit test cases for all the scenario.

@CLAassistant
Copy link

CLAassistant commented Aug 5, 2020 •
edited

CLA assistant check
All committers have signed the CLA.

stefanoScalzo
stefanoScalzo reviewed Aug 17, 2020
View reviewed changes
apps/docs/src/app/core/component-docs/file-uploader/file-uploader-docs.component.html Outdated
<fd-docs-section-title [id]="'file-uploader-compact'" [componentName]="'file-uploader'">
Compact File Upload
</fd-docs-section-title>
<description>Fiori3 recommended file uploader example.</description>
Copy link
Contributor

Choose a reason for hiding this comment

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

Specify what makes this fiori 3 and how to achieve it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i have modified the description.

stefanoScalzo
stefanoScalzo reviewed Aug 17, 2020
View reviewed changes
apps/docs/src/app/core/component-docs/file-uploader/file-uploader-docs.component.html Outdated
<fd-docs-section-title [id]="'file-uploader-compact'" [componentName]="'file-uploader'">
Compact File Upload
</fd-docs-section-title>
<description>Example to demonstrate the compact file uploader. </description>
Copy link
Contributor

Choose a reason for hiding this comment

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

Say what api must be used to achieve compact

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have mentioned it in the description now.

@manu-kr
[ci skip] Fix(File Uploader):Introduncing new fiori3 file uploader co…
8510d41 
…mponent
@manu-kr
[ci skip]Fix:File uploader adding unit test case and review comments fix
e8d0a2b
@manu-kr
[ci skip]Fix:File uploader fixing lint errors
70841b7
@manu-kr
[ci skip] Fix(File Uploader):Introduncing new fiori3 file uploader co…
a9e6fc6 
…mponent
@manu-kr
[ci skip]Fix:(File uploader) changes to fix lint error after rebasing
ecbc2ff
@manu-kr
[ci skip]Fix:(File uploader) changes to fix merge issue
30e7749
@manu-kr
[ci skip] Fix(file uploader): reomve insline style
73bdf3a
@manu-kr
[ci skip] Fix(file uploader): resolve conflicts
3dbb90c
@manu-kr
[ci skip] Fix(file uploader): adding unique id's to example pages
812584f
@manu-kr
Fix(Core)File Uploader: Introducing new file uploader component
4cce9b3
@manu-kr
Fix(File Uploader):Changes contains fix for file uploader review comm…
debf3b4 
…ents given by Edrez and Mikerodonnel
@manu-kr
Fix(File uploader):Adding @hidden annotation
cf77e01
@manu-kr
Fix(File Uploader):fix for build error
dc938ef
@manu-kr
Fix(file uploader):update new style version
f87f62a
@manu-kr
Fix(file uploader):test case fix
a51119e
@manu-kr
Fix(file uploader): remove changelog.md file
dc8a7e9
@manu-kr
Fix(File Uploader): Modifieing the description for compact file uploa…
52c6333 
…der example
@InnaAtanasova InnaAtanasova changed the title Fix(File Uploader):Introducing new Fiori3 file uploader component fix: introduce File Uploader component Aug 18, 2020
@InnaAtanasova InnaAtanasova changed the title fix: introduce File Uploader component feat: introduce File Uploader component Aug 18, 2020
@InnaAtanasova InnaAtanasova merged commit 41670b9 into master Aug 18, 2020
@InnaAtanasova InnaAtanasova deleted the fix/file-uploader branch August 18, 2020 18:38
@manavs manavs added this to the Sprint 44 - ariba milestone Sep 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mikerodonnell89 mikerodonnell89 mikerodonnell89 approved these changes

@JKMarkowski JKMarkowski JKMarkowski approved these changes

@KevinOkamoto KevinOkamoto KevinOkamoto approved these changes

@stefanoScalzo stefanoScalzo stefanoScalzo approved these changes

@sKudum sKudum sKudum approved these changes

@kavya-b kavya-b kavya-b approved these changes

@Lokanathan-k Lokanathan-k Lokanathan-k approved these changes

@DeepakSap14 DeepakSap14 DeepakSap14 approved these changes

@fkolar fkolar Awaiting requested review from fkolar

@droshev droshev Awaiting requested review from droshev

@InnaAtanasova InnaAtanasova Awaiting requested review from InnaAtanasova

@salarenko salarenko Awaiting requested review from salarenko

Assignees

@manu-kr manu-kr

Labels
blocked blocked ticket core Core library specific issues New Fiori component
Projects
None yet
Milestone
Sprint 44 - ariba
Development

Successfully merging this pull request may close these issues.

Create 'File Uploader' component in ngx/core