Skip to content

Introduce a filter to disable the CoBlocks bundled svg icons#1403

Merged
richtabor merged 7 commits into
masterfrom
add/filter-disable-bundled-icons
Mar 9, 2020
Merged

Introduce a filter to disable the CoBlocks bundled svg icons#1403
richtabor merged 7 commits into
masterfrom
add/filter-disable-bundled-icons

Conversation

@EvanHerman
Copy link
Copy Markdown
Contributor

@EvanHerman EvanHerman commented Mar 5, 2020

Resolves #1394, #1405

Description

Introduce a new filter to disable the bundled icons that ship with CoBlocks. This allows users who implement custom icons to remove the excess icons that they may not ever use.

Screenshots

Types of changes

New filter: coblocks_bundled_icons_enabled
Note: I named the filter this way to match the syntax of the typography controls filter, coblocks_typography_controls_enabled.

Usage

add_filter( 'coblocks_bundled_icons_enabled', '__return_false' );

How has this been tested?

Manually tested.

Checklist:

  • My code is tested
  • My code follows accessibility standards
  • My code has proper inline documentation
  • I've included any necessary tests
  • I've included developer documentation
  • I've added proper labels to this pull request

@EvanHerman EvanHerman added [Type] Enhancement Something new that adds functionality [Status] Needs Review Tracking pull requests that need another set of eyes labels Mar 5, 2020
@EvanHerman EvanHerman added this to the Next Release milestone Mar 5, 2020
@EvanHerman EvanHerman self-assigned this Mar 5, 2020
@cypress
Copy link
Copy Markdown

cypress Bot commented Mar 5, 2020



Test summary

6 0 0 0


Run details

Project CoBlocks
Status Passed
Commit cfd8fd1
Started Mar 6, 2020 5:22 PM
Ended Mar 6, 2020 5:22 PM
Duration 00:45 💡
OS Linux Debian - 10.3
Browser Chrome 80

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@richtabor
Copy link
Copy Markdown

Looking good! One last bit: can you add a quick doc article that references this - using this doc page as a template?

After that it's good to merge.

@colorful-tones
Copy link
Copy Markdown

@EvanHerman I downloaded a .zip of your branch add/filter-disable-bundled-icons, ran npm install and npm run build.

I then went to create a new Page and add an Icon block and see this:
Screenshot 2020-03-05 16 42 07

I have add_filter( 'coblocks_bundled_icons_enabled', '__return_false' ); in theme's functions.php and I currently have a series of SVGs in the theme's my-theme/coblocks/icons/*.svg. Am I missing something? Because previously my SVGs were showing alongside CoBlocks icons.

It says the config.json is/was optional, but wondering if this addition makes it a requirement? #1006 (comment)

@EvanHerman
Copy link
Copy Markdown
Contributor Author

EvanHerman commented Mar 5, 2020

@colorful-tones I don't think that there's anything missing, from what you describe. I am also seeing the same error now, but I'm not sure what may have changed.

I didn't notice the error because during my testing I added the icon block to the page, published it, and then added the filter, which removes all of the bundled icons without throwing any errors.

I am going to do some testing and see if I can narrow it down and fix things up. I will post back here when I've figured it out.

@EvanHerman EvanHerman added [Status] In Progress Tracking issues with work in progress and removed [Status] Needs Review Tracking pull requests that need another set of eyes labels Mar 5, 2020
@EvanHerman
Copy link
Copy Markdown
Contributor Author

@colorful-tones I found the issue. Things were breaking when the block was added because the default icons array was set to the icons that we removed through the filter. I've pushed up some code that resets the default svg icons array when the default icons are disabled and custom icons exist.
12c820f#diff-6c0efa8ff85c2dcb3975c2beb5a04fb2R36-R39

Feel free to pull things down and re-test. Let me know if you encounter any issues, and again I can dive back in and we can figure it out.

Thanks!
Evan

@colorful-tones
Copy link
Copy Markdown

@EvanHerman works like a charm! 👏

One thing seemed confusing, but not a blocker to release this IMO. So I opened a new Feature Request #1405

@EvanHerman EvanHerman added [Status] Needs Review Tracking pull requests that need another set of eyes and removed [Status] In Progress Tracking issues with work in progress labels Mar 6, 2020
@richtabor richtabor added [Priority] Low This issue/pull request is not immediate and removed [Status] Needs Review Tracking pull requests that need another set of eyes labels Mar 9, 2020
@richtabor richtabor added the [Status] Ready to Merge Tracking pull requests that are 100% approved and passing, ready to merge into master label Mar 9, 2020
@richtabor richtabor merged commit a5bc156 into master Mar 9, 2020
@richtabor richtabor deleted the add/filter-disable-bundled-icons branch March 9, 2020 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Priority] Low This issue/pull request is not immediate [Status] Ready to Merge Tracking pull requests that are 100% approved and passing, ready to merge into master [Type] Enhancement Something new that adds functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ISNBAT see the icons style panel when custom icons are loaded and no config.json exists ISBAT Filter out and remove CoBlocks icons SVGs

3 participants