Skip to content

Social Menu: Add an SVG option to the menu. Default option still Genericons.#6096

Merged
briancolinger merged 2 commits intomasterfrom
social-menu-svg
Feb 21, 2017
Merged

Social Menu: Add an SVG option to the menu. Default option still Genericons.#6096
briancolinger merged 2 commits intomasterfrom
social-menu-svg

Conversation

@thomasguillot
Copy link
Copy Markdown
Contributor

No description provided.

@jeherve jeherve added [Feature] Theme Tools [Status] Needs Review This PR is ready for review. [Team] tdiv Enhancement Changes to an existing feature — removing, adding, or changing parts of it labels Jan 13, 2017
@jeherve jeherve added this to the 4.6 milestone Jan 13, 2017
@jeherve jeherve modified the milestones: 2/17 - February, 4.7.0 - March 2017 Jan 30, 2017
@samhotchkiss samhotchkiss removed this from the 4.7.0 - March 2017 milestone Feb 3, 2017
@dereksmart
Copy link
Copy Markdown
Contributor

I checked out this branch and things seemed to look ok. @thomasguillot what specifically should I be looking for? Could you please provide some testing instructions here? Thanks.

@thomasguillot
Copy link
Copy Markdown
Contributor Author

@dereksmart when you add support to a theme add_theme_support( 'jetpack-social-menu', 'svg' ); should use the SVG icons instead of the outdated Genericons.

We have a few themes on .com already using this like Dara and Textbook.

* Return the type of menu the theme is using.
*
* @uses get_theme_support()
* @return $menu_type
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please add return type. in this case I think it would be

@return null|string $menu_type


if ( has_nav_menu( 'jetpack-social-menu' ) ) {
wp_enqueue_style( 'jetpack-social-menu', plugins_url( 'social-menu/social-menu.css', __FILE__ ), array( 'genericons' ), '1.0' );
wp_enqueue_style( 'jetpack-social-menu', plugins_url( 'social-menu/social-menu-' . $menu_type . '.css', __FILE__ ), $deps, '1.0' );
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we check to make sure that $menu_type is a valid string (since jetpack_social_menu_get_type() can return null) before we enqueue it?

@dereksmart
Copy link
Copy Markdown
Contributor

Read through the code, but haven't tested it. @thomasguillot are there any .org themes that we can test this with?

@jeherve jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Feb 13, 2017
@thomasguillot
Copy link
Copy Markdown
Contributor Author

@jeherve jeherve added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Feb 14, 2017
@dereksmart dereksmart dismissed their stale review February 20, 2017 14:53

addressed the comments

@dereksmart dereksmart added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Feb 20, 2017
Copy link
Copy Markdown
Contributor

@dereksmart dereksmart left a comment

Choose a reason for hiding this comment

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

Tested with Dara, works well 👍

@briancolinger briancolinger merged commit e5b6c01 into master Feb 21, 2017
@matticbot matticbot removed the [Status] Ready to Merge Go ahead, you can push that green button! label Feb 21, 2017
@briancolinger briancolinger deleted the social-menu-svg branch February 21, 2017 21:20
@jeherve
Copy link
Copy Markdown
Member

jeherve commented Feb 21, 2017

I opened #6469 to address a small issue with the changes.

jeherve added a commit that referenced this pull request Feb 28, 2017
dereksmart pushed a commit that referenced this pull request Feb 28, 2017
* Changelog: update stable tag and move changelog to changelog.txt

Also remove old releases from readme.txt to keep the changelog tab short.

* Changelog: add #5883

Also update the filter's docblock to match new version.

* Changelog: add #5938

* Changelog: add #6298

* Changelog: add #3405

* Changelog: add #5941

* Changelog: add #6239

* Changelog: add #6281

* Changelog: add #6303

* Changelog: add #6018

* Changelog: add #6300

* Changelog: add #6296

* Changelog: add #6130

* Changelog: add #6292

* Readme: remove extra "on".

* Changelog: add #6307

* Changelog: add #3297

* Changelog: add #6275

* Changelog: add #6321

* Changelog: add #6297

* Readme: update the support forum link anchor.

Anchor changed when WordPress.org forums were updated to bbPress 2

* Readme: update list of a12s, it wasn't up to date anymore!

* Changelog: add #6338

* Changelog: add #6337

* Changelog: add #6335

* Changelog: add #6333

* Testing List: first version of the 4.7 testing list.

* Changelog: add #6332

* Changelog: add #6325

* Changelog: add #6326

* Changelog: add #6339

* Changelog: add #6342

* Changelog: add #6343

* Changelog: add #6346

* Changelog: add #6347

* Changelog: add #6279

* Changelog: add #6306

* Changelog: add #6312

* Changelog: add #6316

* Changelog: add #6171

* Changelog: add #6317

* Changelog: add #6246

* Changelog: add #6263

* Changelog: add #4220

* Changelog: add #5888

* Changelog: add #3406

* Changelog: add #3637

* Changelog: add #6320

* Changelog: add #5992

* Changelog: add #6322

* Changelog: add #6324

* Changelog: add #6352

* Changelog: add #6355

* Changelog: add #6360

* Changelog: add #6362

* Changelog: add #6369, #6382

* Changelog: add #6370

* Changelog: add #6375

* Changelog: add #6383

* Changelog: add #6384

* Changelog: add #6386

* Changelog: add #6395

* Changelog: add #6403

* Changelog: add #6406

* Changelog: add #6418

* Changelog: add #6419

* Changelog: add #6434

* Changelog: add #6446

* Changelog: add #6006

* Changelog: add #6096

* Changelog: add #6399

* Changelog: fix typo.

@see #6331 (comment)

* Changelog: add #6440

* Changelog: add #6443

* Changelog: add #6445

* Changelog: add #6463

* Changelog: add #6468

* Changelog: add #6471

* Changelog: add #6474

* Changelog: add #6480

* Changelog: add #6497

* Changelog: add #6499

* Changelog: add #6514

* Changelog: add #6267

* Changelog: add #5940

* Changelog: add #6492

* Changelog: add #5281

* Changelog: add #6327

* Changelog: add #6451

* Changelog: add #6525

* Changelog: add #6530
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Feature] Theme Tools

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants