fix: uploaded svg icon not showing

[Arukuen]

fixes #3103

Details of the issue and the solution: https://www.notion.so/SVG-icon-only-shows-up-on-some-blocks-with-icon-3103-94b9c7925d1f4eaba8ebb2752f4b1660?pvs=4

TLDR: The SVG that can be uploaded by the users might have inline style which takes precedence specially since this can be applied to the lowest level which is the <path> element. This PR fixes the issue by applying the fill directly to <path> to have higher precedence.

[Arukuen]

Will ask API Key to test solution for Map

[github-actions]

🤖 Pull request artifacts

file	commit
pr3179-stackable-3179-merge.zip	43dcf94

[github-actions]

Try this Pull Request in the WordPress playground: https://playground.wordpress.net/?mode=seamless#{"landingPage":"/wp-admin/post-new.php?post_type=page","preferredVersions":{"php":"latest","wp":"latest"},"steps":[{"step":"login","username":"admin","password":"password"},{"step":"installPlugin","pluginZipFile":{"resource":"url","url":"https://raw.githubusercontent.com/gambitph/Stackable/artifacts/pr3179-stackable-3179-merge.zip"}}]}

[andeng1106]

@Arukuen Issue has been fixed on the three blocks affected. However, there's one remaining issue for Map block only:

• Block errors are encountered on Map block when using the pr build. See error in console.
  • Block error happens for existing Map blocks with icons selected from icon library / uploaded an svg icon
  • Steps to replicate:
    1. On 3.12.17, add a Map block
    2. Add a Map Marker
    3. Upload an svg icon or select from icon selection
    4. Save page
    5. Upgrade to pr build
    6. Refresh editor
    7. See block error
  

[Arukuen]

@andeng1106 Since the SVG is serialized for map block, the iconURL of the generated icon will also change when the application of attribute (which this PR did to apply the fill to SVG shapes) is changed. That is why the value of the url property for the data-icon-option for the production build and the PR build are different which causes the issue.

To address this, we can advise affected users to use the "Attempt Block Recovery" feature, which successfully reformats the URL to match the current build and resolves the block error. However, I would like ask for suggestion if there is a better approach.

[bfintal]

@Arukuen since we are encountering a block error that can be recovered, we'll need to add some deprecation code to handle migration of the icon. The expected behavior is for the map block not to encounter an error upon upgrading to this new build

[bfintal]

It's great that deprecation is already handled, but it's also a good idea to further separate our deprecated code so it doesn't mingle with our up-to-date functionality. Currently, our deprecated code is located inside the util.js code:

export const getIconOptions = ( attributes, isDeprecated ) => {
    // ...
    if ( isDeprecated ) {
        // ...

I recommend reverting back to the original, then create a function specific for the needs of the deprecation inside src/block/map/deprecated.js that uses the original getIconOptions instead. This way, all our deprecated code stays out of our main code.

// file: src/block/map/deprecated.js
import { getIconOptions } from './utils'

const getIconOptions_3_13_0 = attributes => {
    const newAttributes = { ...attributes }
    // Do things required for the deprecation...
    return getIconOptions( newAttributes )
}

addFilter( 'stackable.map.util.applySVGAttributes', 'stackable/3.13.0', ( output, attributes, props ) => {
	if ( semverCompare( props.version, '<', '3.13.0' ) ) {
		return getIconOptions_3_13_0( attributes )
	}
	return output
} )

[bfintal]

I recommend changing the name of this filter because if you look at where this filter is being used, it's not related to util.applySVGAttributes. I feel like stackable.map.icon-options makes more sense here:

Suggested change

	'stackable.map.util.applySVGAttributes',
	'stackable.map.icon-options',

Also change the corresponding code that uses this filter