-
Notifications
You must be signed in to change notification settings - Fork 48
Feature/body weight chart filter 1696 #988
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
Feature/body weight chart filter 1696 #988
Conversation
| variant={currentFilter === '' ? 'contained' : 'outlined'} | ||
| sx={{ fontFamily: theme.typography.fontFamily }} | ||
| > | ||
| All |
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.
can you make the labels translatable? Just add them to public/locales/en/translation.json and then you can just use them with {t('key')}. Just add them to the root, these could potentially be also used soemwhere else in the application
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.
Of course! I've updated the code based on your feedback.
src/services/weight.ts
Outdated
| export const getWeights = async (): Promise<WeightEntry[]> => { | ||
| const url = makeUrl(WEIGHT_PATH, { query: { ordering: '-date', limit: 900 } }); | ||
| export const getWeights = async (filter: FilterType = ''): Promise<WeightEntry[]> => { | ||
| const url = makeUrl(WEIGHT_PATH, { query: { ordering: '-date', limit: 900, filter } }); |
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.
sorry I forgot one more! The backend doesn't currently understand the filter parameter, but we can calculate the date based on it and pass it like this 'date_gte': <iso-date-string>
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 have also modified the backend to support the filter and linked the pull request to the issue, alongside this one. Would you prefer the solution I provided, or would you like me to adjust it according to your suggestion and leave the backend unchanged?
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 saw that after I wrote this 😄. I think putting the specific date is better since it's more explicit and we don't need a special syntax for that (you'll need to pull master for this BTW)
|
awesome, thank you! |
Proposed Changes
Related Issue
Please check that the PR fulfills these requirements