-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
feat(manager): server allowance usage component #2311
base: master
Are you sure you want to change the base?
Conversation
daniellacosse
commented
Dec 13, 2024
server_manager/www/views/server_view/server_allowance_usage/index.ts
Outdated
Show resolved
Hide resolved
return html` | ||
<p class="message">${this.message}</p> | ||
<p class="allowance"> | ||
<span class="allowance-percentage">${this.formattedPercentage}</span> |
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.
Add some unit tests to validate the formatted values.
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.
So we don't really have UI unit tests set up yet - well, we have this approach, but yikes
Some options here -
- use the above approach, drilling down via
querySelector
andshadowRoot
- not ideal imo. Klugy, cumbersome, fragile. - set up something better - karma snapshots or storybook test runner. Ideal, but should probably be done in a separate PR.
- Break these out into separate functions and test them that way. Prefer to not let testability determine architecture but would be good enough and kicks the can down the road.
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.
I'm fine with something like querySelector
for now. That's how I tested the contact form logic in the absence of a better testing framework. I really just want something here to prevent regression of this formatting.
@property({type: String}) message: string; | ||
@property({type: Number}) allowanceUsed: number; | ||
@property({type: Number}) allowanceLimit: number; | ||
@property({type: String}) allowanceUnit: 'gigabyte' | 'terabyte' = 'terabyte'; |
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.
nit Shall we be "smart" to determine the unit automatically? For example, just let the caller pass a raw bytes: 1,234,567,000
, then we automatically display it as 1.23 GB
?