Skip to content
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

Add some docs for pattern B #10321

Merged
merged 3 commits into from
Aug 17, 2022
Merged

Add some docs for pattern B #10321

merged 3 commits into from
Aug 17, 2022

Conversation

iwiznia
Copy link
Contributor

@iwiznia iwiznia commented Aug 9, 2022

Details

Fixed Issues

$https://github.com/Expensify/Expensify/issues/221828

No test, no QA, only documentation

@iwiznia iwiznia self-assigned this Aug 9, 2022
@iwiznia iwiznia requested a review from a team as a code owner August 9, 2022 15:30
@melvin-bot melvin-bot bot requested review from puneetlath and removed request for a team August 9, 2022 15:31
This pattern queues the API request, but also makes sure that the user is aware that the request hasn’t been sent yet **when the user is offline**.
When the user is online, the feature should just look like it succeeds immediately (we don't want the offline UI to flicker on and off when the user is online).
When the user is offline:
- Things pending to be created or updated will be shown greyed out (0.6 opacity)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it 0.5 opacity (50%)? 🤔 CC: @shawnborton

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, should be .5!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could've sworn I had it as 0.5 and then changed it to 0.6, but no, I was misremembering

When the user is online, the feature should just look like it succeeds immediately (we don't want the offline UI to flicker on and off when the user is online).
When the user is offline:
- Things pending to be created or updated will be shown greyed out (0.6 opacity)
- Things pending to be deleted will have strikethrough
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be better for the avoidance of doubt to update this one to note that it's also greyed out: "Things pending to be deleted will be greyed out and will have strikethrough"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes

**Handling errors:**
- The OfflineWithFeedback component already handles showing errors too, as long as you pass the error field in the errors prop
- When dismissing the error, the onClose prop will be called, there we need to call an action that either:
- If the pendingAction was `delete`, it removes the data altogether
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NAB: reads better with the bullet above if you switch this sub-bullet around to:

  • removes the data altogether, if the pendingAction was delete

@iwiznia
Copy link
Contributor Author

iwiznia commented Aug 9, 2022

Updated

trjExpensify
trjExpensify previously approved these changes Aug 9, 2022
contributingGuides/OFFLINE_UX.md Outdated Show resolved Hide resolved
Co-authored-by: Puneet Lath <puneet@expensify.com>
@iwiznia
Copy link
Contributor Author

iwiznia commented Aug 12, 2022

Updated with the suggestion, please re-review.

contributingGuides/OFFLINE_UX.md Outdated Show resolved Hide resolved
contributingGuides/OFFLINE_UX.md Outdated Show resolved Hide resolved
@iwiznia iwiznia merged commit 4a4ef54 into main Aug 17, 2022
@iwiznia iwiznia deleted the ionatan_patternbdoc branch August 17, 2022 14:47
@OSBotify
Copy link
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @iwiznia in version: 1.1.89-0 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants