[RNMobile] Social icons #23017
[RNMobile] Social icons #23017
Conversation
lukewalczak
added
the
[Block] Social
|
Size Change: +19 B (0%) Total Size: 1.14 MB
ℹ️ View Unchanged
|
lukewalczak
force-pushed
the
rnmobile/social_links
branch
from
b091d24
to
a0edba7
Compare
lukewalczak
force-pushed
the
rnmobile/social_links
branch
from
b7a46c7
to
631daee
Compare
packages/block-editor/src/components/block-list/index.native.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/block-list/index.native.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
LGTM! Nice work @lukewalczak! 👏 Tested it on the demo app (iOS/Android) and also on WordPress iOS.
| @@ -434,7 +434,8 @@ export function createBlockWithFallback( blockNode ) { | |||
| if ( name && name.indexOf( 'core/social-link-' ) === 0 ) { | |||
| // Capture `social-link-wordpress` into `{"service":"wordpress"}` | |||
| attributes.service = name.substring( 17 ); | |||
| name = 'core/social-link'; | |||
| // Display social link service name for mobile platform | |||
| name = Platform.OS === 'web' ? 'core/social-link' : name; | |||
There was a problem hiding this comment.
Can you help me understand why the deprecated name needs to be kept in mobile?
My worries:
- Even though we should keep this compat shim for years to preserve older content, the idea is still to help move content forward by remapping block names, so that, when a user edits an old post, the Social Link markup is correctly updated.
- The shim is likely to move out of the parser and be placed somewhere more sustainable (e.g. external compat file). This could be provided by JS filters or some other form of hooking. Once we do that, it would be important that the shim unconditionally reassigns
nametocore/social-link.
/cc @lukewalczak @hypest
There was a problem hiding this comment.
Hey @mcsf, that change was basically needed to display specific social icon name and its icon within toolbar on the mobile platform.
Without this change, it would be confusing as each social-icon would have the same label and the same icon after saving the post.
| without parser change | with parser change |
|---|---|
 |
 |
The introduced change was the easiest solution to fulfil the requirement, but I understand your concerns and will double-check if we can achieve the same effect without parsers changes.
There was a problem hiding this comment.
However, could you please elaborate more on:
The shim is likely to move out of the parser and be placed somewhere more sustainable (e.g. external compat file). This could be provided by JS filters or some other form of hooking. Once we do that, it would be important that the shim unconditionally reassigns name to core/social-link
There was a problem hiding this comment.
Thanks for the context.
It's clear then that support for Block Variations needs to be improved so that those interactions can take into account attributes like a variation's title or icon.
For comparison, even on the Web we let the variation show in certain places and let the base type show in others, which is also a problem but less confusing than in the apps:
See open questions about variations: #16283 (comment)
However, could you please elaborate more on:
The shim is likely to move out of the parser […]
Sure. I'm referring to this comment by Grzegorz.
Context:
- Block variations API: Variations (formerly patterns) API for blocks #16283, Blocks: Match blocks in the inserter using keywords from patterns #19243
- Social Icons implementation: Social Link: group all variations under one block type #19887
There was a problem hiding this comment.
Correct me if I'm wrong: I assume that the main goal is to revert that change and have equal HTML on both platforms?
There was a problem hiding this comment.
I've created a PR fixing that, please check it ✌️
Description
NOTE: That PR should be merged after #23299 since it's using theLinkSettingscomponent coming from there.NOTE: That PR should be merged after #23465That PR introduces a new block called
Social Icon.Testing:
How has this been tested?
Social IconsblockExpect: Social Icons is added with 4 icons, where WordPress icon is already fulfilled with the link and it's active (has product color)
Facebookiconlinkicon in mobile toolbar above the keyboardURLeg. facebook.com andLabelExpect: Facebook social icon is active (has product color)
Social IconsExpect: Only active social icons left
Screenshots
Types of changes
New feature:
Social IconsblockChecklist: