Skip to content

Social Icon: Try auto-matching variation based on URL #59303

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.

Sign up for GitHub

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

Closed
Mamaduka wants to merge 10 commits into from
Closed

Social Icon: Try auto-matching variation based on URL #59303

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
d0070f0
Social Icon: Try auto-matching variation based on URL
Mamaduka Feb 22, 2024
0488a5d
Remove old test fixture
Mamaduka Feb 23, 2024
db29dfd
Only match to 'https' protocol
Mamaduka Feb 26, 2024
e153792
Fix service logic
Mamaduka Mar 21, 2024
6c2b7a7
Match both x.com and twitter.com to X service
Mamaduka Mar 21, 2024
142a9a7
Update unit tests
Mamaduka Mar 21, 2024
567c762
Fix typos
Mamaduka Mar 28, 2024
f923784
Don't add 'wp-social-link-undefined' class
Mamaduka Mar 28, 2024
7801994
Fix rebase error
Mamaduka May 15, 2024
4b67c7d
Use relaxed isURLLike and ensure https is always prepended on submit
getdave Aug 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Don't add 'wp-social-link-undefined' class
  • Loading branch information
@Mamaduka @getdave
Mamaduka authored and getdave committed Aug 30, 2024
commit f92378497274138eac5e06e0539dc117def6b8fe
3 changes: 2 additions & 1 deletion packages/block-library/src/social-link/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,8 @@ const SocialLinkEdit = ( {
iconBackgroundColor,
iconBackgroundColorValue,
} = context;
const classes = clsx( 'wp-social-link', 'wp-social-link-' + service, {
const classes = clsx( 'wp-social-link', {
[ `wp-social-link-${ service }` ]: !! service,
'wp-social-link__is-incomplete': ! url,
Copy link
Contributor

@afercia afercia Mar 26, 2024

Choose a reason for hiding this comment

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

The way this 'is-incomplete' CSS class works should be adjusted.
When the UI shows the 'link' icon button as a placeholder, the button is actually an actionable control. That is fine but it looks 'disabled' because, technically, it doesn't have the url prop set.
However, if we want to use this button as an actionable, enabled, focusable control then it can't look 'disabled'.

Copy link
Member Author

@Mamaduka Mamaduka Mar 28, 2024

Choose a reason for hiding this comment

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

I think we can remove this class. With update flow, the child block is either a placeholder or matches a service. What do you think?

[ `has-${ iconColor }-color` ]: iconColor,
[ `has-${ iconBackgroundColor }-background-color` ]:
Expand Down