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(image): add image via.placeholder provider #1186

Merged
merged 22 commits into from Aug 8, 2022
Merged

feat(image): add image via.placeholder provider #1186

merged 22 commits into from Aug 8, 2022

Conversation

Devdre1909
Copy link
Contributor

@Devdre1909 Devdre1909 commented Jul 24, 2022 •
edited

Issue #1111
Add support for https://placeholder.com/

width, height, text, format, background color, and text color supported

@Devdre1909 Devdre1909 requested a review from a team as a code owner July 24, 2022 10:42
@xDivisionByZerox xDivisionByZerox added c: feature Request for new feature p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug labels Jul 24, 2022
@codecov
Copy link

codecov bot commented Jul 24, 2022 •
edited

Codecov Report

Merging #1186 (5dbb164) into main (7372e9a) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 5dbb164 differs from pull request most recent head ec77e3b. Consider uploading reports for the commit ec77e3b to get more accurate results

@@           Coverage Diff            @@
##             main    #1186    +/-   ##
========================================
  Coverage   99.63%   99.63%            
========================================
  Files        2155     2156     +1     
  Lines      236884   236988   +104     
  Branches      994     1007    +13     
========================================
+ Hits       236020   236124   +104     
  Misses        843      843            
  Partials       21       21
Impacted Files Coverage Δ
src/modules/image/index.ts 100.00% <100.00%> (ø)
src/modules/image/providers/placeholder.ts 100.00% <100.00%> (ø)

import-brain
import-brain reviewed Jul 24, 2022
View reviewed changes
Copy link
Member

@import-brain import-brain left a comment

Choose a reason for hiding this comment

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

We should specify that width and height are in pixels.

src/modules/image/providers/placeholder.ts Outdated Show resolved Hide resolved
src/modules/image/providers/placeholder.ts Outdated Show resolved Hide resolved
@Devdre1909
feat(image)
f292381 
- include . after jsdoc comments
- specifiy that width and height is in pixels
ST-DDT
ST-DDT requested changes Jul 25, 2022
View reviewed changes
src/modules/image/providers/placeholder.ts Show resolved Hide resolved
src/modules/image/providers/placeholder.ts Outdated Show resolved Hide resolved
test/image.spec.ts Outdated Show resolved Hide resolved
ST-DDT
ST-DDT requested changes Jul 27, 2022
View reviewed changes
src/modules/image/providers/placeholder.ts Outdated Show resolved Hide resolved
src/modules/image/providers/placeholder.ts Outdated Show resolved Hide resolved
src/modules/image/providers/placeholder.ts Outdated Show resolved Hide resolved
src/modules/image/providers/placeholder.ts Outdated Show resolved Hide resolved
src/modules/image/providers/placeholder.ts Outdated Show resolved Hide resolved
src/modules/image/providers/placeholder.ts Show resolved Hide resolved
src/modules/image/providers/placeholder.ts Outdated Show resolved Hide resolved
src/modules/image/providers/placeholder.ts Outdated Show resolved Hide resolved
Devdre1909 and others added 5 commits July 27, 2022 18:13
@Devdre1909 @ST-DDT
Update src/modules/image/providers/placeholder.ts
c2352b9 
Co-authored-by: ST-DDT <ST-DDT@gmx.de>
@Devdre1909 @ST-DDT
Update src/modules/image/providers/placeholder.ts
bbcfb39 
Co-authored-by: ST-DDT <ST-DDT@gmx.de>
@Devdre1909 @ST-DDT
Update src/modules/image/providers/placeholder.ts
c5943c9 
Co-authored-by: ST-DDT <ST-DDT@gmx.de>
@Devdre1909 @ST-DDT
Update src/modules/image/providers/placeholder.ts
dcb509a 
Co-authored-by: ST-DDT <ST-DDT@gmx.de>
@Devdre1909 @ST-DDT
Update src/modules/image/providers/placeholder.ts
8e2c687 
Co-authored-by: ST-DDT <ST-DDT@gmx.de>
@xDivisionByZerox xDivisionByZerox added the m: image Something is referring to the image module label Jul 28, 2022
@ST-DDT
Copy link
Member

ST-DDT commented Jul 29, 2022

Please update the tests so the dont fail anymore.

@Shinigami92 Shinigami92 added this to the v7 - Current Major milestone Jul 30, 2022
@ST-DDT
Copy link
Member

ST-DDT commented Aug 5, 2022

@Devdre1909 May I go ahead and fix these things to get this merged or do you wish to do this yourselves?

@Devdre1909
Copy link
Contributor Author

@Devdre1909 May I go ahead and fix these things to get this merged or do you wish to do this yourselves?

I will.
Thank you.

@Shinigami92 Shinigami92 changed the title feat(image): Add Image placeholder module feat(image): add image via.placeholder provider Aug 8, 2022
@Shinigami92 Shinigami92 enabled auto-merge (squash) August 8, 2022 14:39
@Shinigami92 Shinigami92 enabled auto-merge (squash) August 8, 2022 16:39
@Shinigami92 Shinigami92 merged commit 00d4741 into faker-js:main Aug 8, 2022
@xDivisionByZerox xDivisionByZerox mentioned this pull request Apr 25, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Minozzzi Minozzzi Minozzzi left review comments

@import-brain import-brain import-brain left review comments

@xDivisionByZerox xDivisionByZerox xDivisionByZerox left review comments

@ST-DDT ST-DDT ST-DDT approved these changes

@Shinigami92 Shinigami92 Shinigami92 approved these changes

Assignees

@Devdre1909 Devdre1909

Labels
c: feature Request for new feature m: image Something is referring to the image module p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug
Projects
No open projects
Faker Roadmap
Status: Done
Milestone
v7 - Current Major
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@Devdre1909 @ST-DDT @Shinigami92 @Minozzzi @import-brain @xDivisionByZerox