-
-
Notifications
You must be signed in to change notification settings - Fork 919
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
faker.image.dataUri is not random #2307
Comments
Possible options
|
Making the color random if it is not provided is a great idea. I also like the other two ideas. A quite similar option would be to add a random shape somewhere in the SVG. |
I created an issue for the (other) image functions as well. |
better idea we could put a random emoji in the middle instead of a random word, then its locale indepdendent and you don't need to worry about the text being too long. |
Hey @ST-DDT this my first time contributing to a Open Source Project |
Close but we dont use Math.random because we want reproducible values. So we usually use faker.helpers.arrayElement or faker.number.int for that. For this case specifically we already have a method that can generate rgb colors: |
I would probably inline that and omit the args matching the defaults but yes. |
It supports all |
Please Check my PR: #2316 |
Any thoughts on including an emoji or a random shape/line? if we're going to do changes to this method to make it more random may as well do it all in one PR. |
I think we could randomly use one of these overlays:
(plus a new textColor, featureColor, or whateverColor option) |
if we omit random word it will still work in the |
Well, if we argue like that then what if we don't have any locale data? Would the "feature"/"overlay" replace the current dimension text? (or would it be a 4th option?) |
think the dimenson text is useful so i'd probably do it next to / below / above the existing dimension text |
faker.image.dataUri is not random, it always returns the same image (a grey rectangle with the provided width/height in text)
https://stackblitz.com/edit/faker-js-demo-quium5?file=index.ts
Source: https://github.com/faker-js/faker/blob/651d1a8c/src/modules/image/index.ts#L373
I also noted that the width x height text doesn't fit for small
width
parametersOriginally posted by @ST-DDT in #2273 (comment)
The text was updated successfully, but these errors were encountered: