-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[Hold for payment 2024-08-27][$250] Missing checkmark when reselecting date of month in Submission frequency modal #47514
Comments
Triggered auto assignment to @lschurr ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.The selected date is missing a checkmark on the right. What is the root cause of that problem?Currently, we are saving
What changes do you think we should make in order to solve the problem?We should convert text: toLocaleOrdinal(day),
keyForList: day.toString(), // we have to cast it as string for <ListItem> to work
+ isSelected: day === Number(offset),
isNumber: true, What alternative solutions did you explore? (Optional) |
@lschurr I'm reporter so I can take this bug as C+ |
Edited by proposal-police: This proposal was edited at 2024-08-15 18:53:25 UTC. ProposalPlease re-state the problem that we are trying to solve in this issue.Missing checkmark when reselecting date of month in Submission frequency modal What is the root cause of that problem?We are using unnecessarily using App/src/libs/actions/Policy/Policy.ts Line 453 in a07cce4
What changes do you think we should make in order to solve the problem?We should remove the the We should also update
What alternative solutions did you explore? (Optional)
|
Proposal Updated
|
Job added to Upwork: https://www.upwork.com/jobs/~0101f6414948e6716e |
Current assignee @dukenv0307 is eligible for the External assigner, not assigning anyone new. |
ProposalPlease re-state the problem that we are trying to solve in this issue.Checkmark missing when reselecting date in Submission Frequency modal. What is the root cause of that problem?This comparison has might have different type
What changes do you think we should make in order to solve the problem?If we check the AutoReportingOffset type it can contain number or string Line 1251 in 5a55cd5
We can use == instead of === to compare. Using == instead of === will perform type coercion, which means it will convert the operands to the same type before making the comparison. What alternative solutions did you explore? (Optional)convert both to string for isSelected check. src/pages/workspace/workflows/WorkspaceAutoReportingMonthlyOffsetPage.tsx ....
const dayString = day.toString();
const offsetString = offset.toString();
return {
text: toLocaleOrdinal(day),
keyForList: dayString, // we have to cast it as string for <ListItem> to work
isSelected: dayString === offsetString,
isNumber: true,
}; |
@Krishna2323 If we remove |
@dukenv0307 yes. |
Thank you @Krishna2323. It looks good and tests well on my side. Let's go with @Krishna2323's proposal 🎀👀🎀 C+ reviewed |
Triggered auto assignment to @carlosmiceli, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
📣 @dukenv0307 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app! |
📣 @Krishna2323 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
@dukenv0307, PR ready for review ^ |
@lschurr, this was deployed to production 2 weeks ago. Please process the payments when you have a chance. Thanks |
Thanks for the bump @Krishna2323 - Looks like the automation failed so this should have been paid on August 27th. |
Payment summary:
|
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: v9.0.20-6
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @dukenv0307
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1723741251007689
Action Performed:
Expected Result:
The selected date is still focused and has a checkmark.
Actual Result:
The selected date is missing a checkmark on the right.
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Screen.Recording.2024-08-16.at.00.01.29.mov
Recording.447.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @The text was updated successfully, but these errors were encountered: