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

Refactor bw_dash and bw_github to use the shared library for class OIK_SVG_icons #46

Open
6 tasks done
bobbingwide opened this issue Nov 11, 2021 · 11 comments
Open
6 tasks done
Assignees

Comments

@bobbingwide
Copy link
Owner

bobbingwide commented Nov 11, 2021

For improved front end performance of the [bw_follow_me] shortcode I changed the logic to support SVG icons rather than
dashicons or genericons. Use theme=svg to choose this.

I implemented a shared library class OIK_SVG_icons for the purpose.

Requirements

  • Improve the implementation of SVG icons for the [bw_dash] shortcode.
  • Implement SVG icons for the [bw_github] shortcode.

Proposed solution

  • Implement the shared library class-oik-svg-icons
  • Change [bw_github] to use this shared library rather than genericons
  • Change [bw_dash] to use the shared library logic
  • Update SVG definitions to latest from Gutenberg's icons
  • Eliminate redundant logic
  • Update the Dashicons block to reflect new icons.

See also bobbingwide/oik#187 for notes about the [github] shortcode

@bobbingwide
Copy link
Owner Author

Update SVG definitions to latest from Gutenberg's icons

There are now 620 or so icons. Manually checking the SVG definitions could take a long time.
So it makes sense to automate the process.
See bobbingwide/txt2md#7 for a solution that generates a PHP file containing the accumulated
definitions of the SVG files.

@bobbingwide
Copy link
Owner Author

I've updated the dashiconlist.js with the additional 70 or so Dashicons by extending svg2php.php ( see txt2md ) to echo the required JavaScript, then commenting out the icons which aren't dashicons.

Now I'm looking for an appropriate icon selection tool. I found this issue which is still open.
WordPress/gutenberg#16484

It appears that Gutenberg core doesn't yet have an IconSelect component
I'll have a look at the icon blocks in: CoBlocks, GhostKit, Kioken-blocks
And or look at how the block inserter works.

@bobbingwide
Copy link
Owner Author

I also changed the code so that the blocks are rendered server side from the SVG rather than dashicons.css and font.
I couldn't figure out how to implement the deprecated logic. Old blocks therefore break.

In the editor the block only supports icons which are dashicons.
The number of icons supported by the block is far fewer than those supported by [bw_dash].
I haven't worked out an easy/sensible way to include all the icons from Gutenberg's icon library and Social icons library.

Once done I should be able to extend the logic to support block icons.

@bobbingwide
Copy link
Owner Author

Getting this code to work took a lot of effort. The icon parameter is
either a string - for a dashicon - or an object containing an icon and a name, where the icon is an icon imported from @wordpress/icons.

 renderIcon( icon) {
    	var key = icon && icon.name ? icon.name : icon;
    	var iconValue = icon && icon.icon ? <Icon icon={icon.icon} /> : <Icon icon={icon} />
		return( <li key={key}>{iconValue}{key} </li> );
	}

I'd previously been trying to use.
I don't understand why it didn't work.

var key = icon && icon.name ? icon.name : icon;
var iconValue = icon && icon.icon ? icon.icon : icon;
return( <li key={key}><Icon icon={iconValue} />{key} </li>

@bobbingwide
Copy link
Owner Author

I couldn't figure out how to implement the deprecated logic. Old blocks therefore break.

I was helped by Glen Davies who showed some example code. His comment for the save() method showed me what I was doing wrong.

... the old save method, even though it seems like it is not
needed, as removing save method does not seem to invalidate block,
It is probably worth adding as an historical record

The key word being old.

It seemed to work for some blocks but not for those with Additional CSS class(es) or other versions of the block that was created along the way which may have been created using flaky code that's since been updated.

@bobbingwide
Copy link
Owner Author

bobbingwide commented Feb 28, 2022

After much battling I have finally discovered how to use the CustomSelectControl to create a simple selection list for SVG icons.
In this screenshot there are two Icon selection lists:

  1. ComboboxControl
  2. CustomSelectControl

The logic to set the text colour and size doesn't work on the server. This may need CSS styling and/or extra attribute handling.
It's not right in the editor either

  1. The Verse icon has a very large font - this is applied to the div not the SVG
  2. The text color also applies to the div
  3. It works for the heart icon since in the editor this is still supported using Dashicon logic.

image

@bobbingwide
Copy link
Owner Author

bobbingwide commented Mar 5, 2022

The Verse icon has a very large font - this is applied to the div not the SVG

Try replacing the Font size with a Size control for the SVG.

OK. That seems to work.
image

But...

  • The icon size is not shown in the RangeControl slider
  • The display of the SVG icons in the CustomSelectControl is well iffy

I managed to get the CSS to colour the SVG icons.
This also works in the server rendering.

  • I had to change block.json to tell it about the CSS file.
  • In the previous version the CSS was only being enqueued for the first block that's registered in the server - oik-csv.
  • I'll have to test what gets enqueued for the other blocks.

Now to apply the size logic to the server rendering.

@bobbingwide
Copy link
Owner Author

Note: When the label for the RangeControl was Size the control was really narrow.
It's better with a longer label.
image

image

bobbingwide added a commit that referenced this issue Mar 8, 2022
bobbingwide added a commit that referenced this issue Mar 8, 2022
bobbingwide added a commit that referenced this issue Mar 8, 2022
bobbingwide added a commit that referenced this issue Mar 8, 2022
@bobbingwide
Copy link
Owner Author

Delivered in oik-bob-bing-wide v2.2.1

@bobbingwide
Copy link
Owner Author

In the list of SVG dash icons produced to display the bw_dash shortcode’s examples there are 3 icons which are not visible.

  1. line-dashed-icon
  2. line-dotted-icon
  3. line-solid-icon

They're styled correctly when displayed by the Dashicon block.
There's some CSS that's missing.

svg.svg { fill: currentColor;  fill-rule: evenOdd;}

Add this as inline CSS when inside the bw_dash shortcode and when it's an SVG icon. New function bw_dash_inline_style_svg()

@bobbingwide bobbingwide reopened this Mar 10, 2022
@bobbingwide
Copy link
Owner Author

bobbingwide commented Mar 10, 2022

The plus-circle-icon is also invisible in both the rendered block and the shortcode.

<div class="wp-block-oik-bbw-dashicon"><svg class="svg_plus-circle-icon svg " width="24" height="24" xmlns="http://www.w3.org/2000/svg" viewBox="-2 -2 24 24">
		<path d="M10 1c-5 0-9 4-9 9s4 9 9 9 9-4 9-9-4-9-9-9zm0 16c-3.9 0-7-3.1-7-7s3.1-7 7-7 7 3.1 7 7-3.1 7-7 7zm1-11H9v3H6v2h3v3h2v-3h3V9h-3V6zM10 1c-5 0-9 4-9 9s4 9 9 9 9-4 9-9-4-9-9-9zm0 16c-3.9 0-7-3.1-7-7s3.1-7 7-7 7 3.1 7 7-3.1 7-7 7zm1-11H9v3H6v2h3v3h2v-3h3V9h-3V6z"></path>
	</svg></div>

but it is visible in the CustomSelectControl.
where the SVG is:

/**
 * WordPress dependencies
 */
import { SVG, Path } from '@wordpress/primitives';

const plusCircle = (
	<SVG xmlns="http://www.w3.org/2000/svg" viewBox="-2 -2 24 24">
		<Path d="M10 1c-5 0-9 4-9 9s4 9 9 9 9-4 9-9-4-9-9-9zm0 16c-3.9 0-7-3.1-7-7s3.1-7 7-7 7 3.1 7 7-3.1 7-7 7zm1-11H9v3H6v2h3v3h2v-3h3V9h-3V6zM10 1c-5 0-9 4-9 9s4 9 9 9 9-4 9-9-4-9-9-9zm0 16c-3.9 0-7-3.1-7-7s3.1-7 7-7 7 3.1 7 7-3.1 7-7 7zm1-11H9v3H6v2h3v3h2v-3h3V9h-3V6z" />
	</SVG>
);

export default plusCircle;

Explanation

This is due to the fill-rule: evenodd property being applied to the SVG.
If this is removed then the problem goes away.

https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/fill-rule

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant