-
Notifications
You must be signed in to change notification settings - Fork 2
feat: new BADGE function #20
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
Conversation
sww314
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want to return empty string if there data is not right.
This follows the logic in most "badge" components.
src/functions.js
Outdated
| const length = array?.length || 0; | ||
| return `${prefix}${length}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if the length is 0 or undefined return nothing.
Also handle type error (swallow the error) if they pass in an object with out a length.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sww314 I forgot you mentioned it should be empty rather than 0. Changed in aa58c18
- If an object without a length is passed in, the
?.lengthalready returns undefined and then the|| ''will return empty. Do I need to add more handling? I thought this would handle every undefined case - For
BADGE("A String"), given this is a length calculation, it makes sense to me that it returns the length of the string - For
BADGE({"a": "1", "b": "2"})the result is empty string. The object has no length so it defaulted to empty
sww314
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not show anything with length == 0 (or if object is undefined).
If we want, we could add a showZero prop... but I am not sure it is needed.
https://mui.com/material-ui/react-badge/
Also if you pass in:
BADGE("A String")
You will get an answer. Maybe that is fine.
What happens if you pass in:
BADGE({"a": "1", "b": "2"})
sww314
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for completeness should we add postfix?
BADGE(values, "[", "]") --> [5]
src/functions.js
Outdated
| const length = array?.length || ''; | ||
| return `${prefix}${length}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| const length = array?.length || ''; | |
| return `${prefix}${length}`; | |
| const length = array?.length || ''; | |
| if length: | |
| return `${prefix}${length}`; | |
| return ''; |
make it easy to return nothing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added postfix modified logic in in ab2c955
This does modify the behavior tho: in my previous code it would still show the pre/postfix with an empty value, whereas now it doesn't. Seems fine either way but probably more correct to display nothing
The badge function provides an easy way to get the length of a variable (usually an array), and include an optional text prefix, such as
media: 4when counting number of media objects. It needs to handle empty and undefined values as well without erroring