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

PLANET-4540 Fix facebook share and remove unused code #182

Merged
merged 1 commit into from Dec 19, 2019

Conversation

Inwerpsel
Copy link
Contributor

  • Facebook share now opens in new tab, providing the
    social_media_message as quoted text
  • All SocialMediaCards code was initially copied from the Gallery
    block as a starting point, however this was never cleaned up. This
    removes some of it, except for the focal points, which don't work but it
    is not clear whether it should be removed or the functionality should be
    implemented.
  • Remove the conversion from shortcode.
  • Prevent notices for the link_new_tab fix

@Inwerpsel Inwerpsel requested review from a team, pablocubico and sagarsdeshmukh and removed request for a team December 16, 2019 16:05
@Inwerpsel Inwerpsel force-pushed the PLANET-4540 branch 3 times, most recently from ad3e466 to 114ce22 Compare December 17, 2019 13:27
@@ -243,7 +243,7 @@ function empty_string_to_false_in_link_new_tab_in_columns_blocks( $block ): arra
// Yes, that's right, WordPress doesn't follow its own rules here so we have a camel among snakes.
if ( 'planet4-blocks/columns' === $block['blockName'] ?? null ) {
foreach ( $block['attrs']['columns'] ?? [] as $key => $column ) {
if ( true !== $column['link_new_tab'] ) {
if ( isset( $column['link_new_tab'] ) && true !== $column['link_new_tab'] ) {
Copy link
Member

Choose a reason for hiding this comment

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

Good catch 👍

@sagarsdeshmukh
Copy link
Member

PR looks good to me, the changes works as per JIRA ticket req.

General finding -

  • The remove selected image option not visible.
  • The title and desc not display in admin editor preview
  • The textdomain value is different at some places
  • The label of field are "Select Gallery Images" , "Select gallery image focal point" which not relevant to 'Social media card' block(its mainly because of copy paste from gallery block)

* Facebook share now opens in new tab, providing the
social_media_message as quoted text
* All SocialMediaCards code was initially copied from the Gallery
block as a starting point, however this was never cleaned up. This
removes some of it, except for the focal points, which don't work but it
is not clear whether it should be removed or the functionality should be
implemented.
* Remove the conversion from shortcode.
* Prevent notices for the link_new_tab fix
@Inwerpsel
Copy link
Contributor Author

PR looks good to me, the changes works as per JIRA ticket req.

General finding -

  • The remove selected image option not visible.
  • The title and desc not display in admin editor preview
  • The textdomain value is different at some places
  • The label of field are "Select Gallery Images" , "Select gallery image focal point" which not relevant to 'Social media card' block(its mainly because of copy paste from gallery block)

Yeah I saw some of those issues as well, but it's so much that it deserves its own ticket. Perhaps we can take those remarks along when we fix the focal point behavior.

@comzeradd comzeradd changed the base branch from release/v0.7 to release/v0.7.1 December 17, 2019 18:35
@sagarsdeshmukh sagarsdeshmukh merged commit c2bb55e into release/v0.7.1 Dec 19, 2019
@sagarsdeshmukh sagarsdeshmukh deleted the PLANET-4540 branch December 19, 2019 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants