[RNMobile] Add OfflineStatus component #56934
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This code is non-functioning currently.
This bridge will be required for the planned JavaScript Hook to monitor connection status.
Provides React Hook for monitoring the network connection status via the bridge to the host app.
This reverts commit a8d3660.
Semicolon is unnecessary. Co-authored-by: Tanner Stokes <tanner.stokes@automattic.com>
This bridge enables monitoring the connection status on Android.
Allow the Android platform to request the current network connection status.
The Demo editor fails to build without a mocked bridge method.
derekblank
added
[Type] Enhancement
|
Size Change: +1.03 kB (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 5b32144. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7201205711
|
|
@dcalhoun This PR updates the offline icon provided by the design team, which appears to resolve the vertical centering issue mentioned here. 
|
|
In an attempt to further debug the Android dark/light mode issue mentioned here, I compared against the Paywall block dark/light color implementation, which are identical to the colors and hooks used in the OfflineStatus component. I noted that this issue also affects the Paywall block in the same way. See example video: Paywall Block Video ExamplePaywall.block.movFor both, we will likely need to implement a solution to accomodate theme styles like was done on RichText placeholderTextColor. |
dcalhoun
reviewed
Dec 11, 2023
There was a problem hiding this comment.
In an attempt to further debug the Android dark/light mode issue mentioned here, I compared against the Paywall block dark/light color implementation, which are identical to the colors and hooks used in the OfflineStatus component. I noted that this issue also affects the Paywall block in the same way.
I debugged further. This issue appears to be that the color values used in both OfflineStatus and the aforementioned block are based upon opacity, specifically 8-digit hexadecimal colors.
For both, we will likely need to implement a solution to accomodate theme styles like was done on RichText placeholderTextColor.
From my perspective, the illegible text means we should not use these opacity-based colors for anything that is displayed atop the editor canvas, whose background color we cannot predict due to the current WYSIWYG feature driven by theme colors. I do not think we need to implement JavaScript logic to accommodate theme styles. WDYT?
I did have a thought that it could possibly be related to the hexadecimal alpha values, but hadn't made it there debugging yet. From preliminary testing, using six-digit hex values should resolve the issue. I'll update, and thanks for testing. 👍 |
The current Jest module resolution configuration will fail when there is only a native-specific file without a web-specific file, e.g. only a `style.native.scss` and no sibling `style.scss` next to it. Explicitly importing the native-specific file avoids the error as Jest does not attempt to seek the non-suffixed file.
dcalhoun
approved these changes
Dec 13, 2023
There was a problem hiding this comment.
I retested the styles atop dark editor backgrounds, they are now far more legible. Additionally, I pushed a small import path fix to resolve a Jest test module resolution error.
I believe this is ready to merge and is hidden behind a __DEV__ flag while we work to finish related user-facing features. 🚀
The native implementation is not available within the JavaScript testing environment.
artemiomorales
pushed a commit
that referenced
this pull request
Jan 4, 2024
* feat: Frame useNetInfo hook foundation This code is non-functioning currently. * feat: Add iOS connection status bridge utilities This bridge will be required for the planned JavaScript Hook to monitor connection status. * feat: Add `useIsConnected` hook Provides React Hook for monitoring the network connection status via the bridge to the host app. * Revert "feat: Frame useNetInfo hook foundation" This reverts commit a8d3660. * refactor: Align with project Swift syntax Semicolon is unnecessary. Co-authored-by: Tanner Stokes <tanner.stokes@automattic.com> * feat: Add Android connection status bridge utilities This bridge enables monitoring the connection status on Android. * feat: Android network connection status request utility Allow the Android platform to request the current network connection status. * fix: Add missing `requestConnectionStatus` bridge method mock The Demo editor fails to build without a mocked bridge method. * Add mobile OfflineStatus component * Add OfflineStatus component to block-list behind __DEV__ flag * Replace offline icon and update OfflineStatus text alignment * Update BEM syntax for OfflineStatus * Update OfflineStatus component colors * test: Import sole native stylesheet to fix test module resolution error The current Jest module resolution configuration will fail when there is only a native-specific file without a web-specific file, e.g. only a `style.native.scss` and no sibling `style.scss` next to it. Explicitly importing the native-specific file avoids the error as Jest does not attempt to seek the non-suffixed file. * test: Mock `useIsConnected` native bridge method The native implementation is not available within the JavaScript testing environment. --------- Co-authored-by: David Calhoun <github@davidcalhoun.me> Co-authored-by: Tanner Stokes <tanner.stokes@automattic.com>
4 tasks
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Adds a mobile component to display a visual indicator when working offline within the mobile editor. (Note: the functionality to subscribe to network status is handled in #56861.)
See related discussions in a previous attempt.
Why?
Overall, to improve the user's experience when uploading media.
Addresses:
How?
Adds OfflineStatus component and associated icon/style files.
Testing Instructions
The component is included in block-list behind a dev flag. To test this PR, build the app locally and turn on Airplane Mode while using the editor.
Test Diff
Screenshots or screencast