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

Bypass empty BU Text widgets in class count increment via filter #96

Open
wants to merge 18 commits into
base: develop
Choose a base branch
from

Conversation

alana29s
Copy link
Contributor

@alana29s alana29s commented Sep 21, 2018

Fixes #45

Changes proposed in this pull request

  • Create filter responsive_is_widget_empty to help check for empty widgets.
  • Add filter to check for empty BU Text widgets.
  • Add filter to check for empty BU Links widgets.
  • Make filter fire only if BU Text Widget is active keeping in mind goal of open sourcing this theme and general performance.
  • This will prevent the Widget Class Count from increasing if a widget is empty.

Note: It would be preferable to update the plugins, however, the legacy code and impact of that change is large. This is a work around until BUText Widgets and BU Link Widgets are eventually deprecated in favor of Gutenberg.

Review checklist

[X] I've reviewed the contribution guidelines.
[X] I've updated CHANGELOG.MD with a brief explanation of the changes in this pull request in the unreleased section.
[X] My code follows BU Coding Standards.

@alana29s alana29s self-assigned this Sep 21, 2018
*
* @params array $params An array of widget options.
*/
$is_widget_empty = apply_filters( 'responsive_is_widget_empty', $is_widget_empty = false, $params );
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want to specify $is_bu_widget_empty, or do you think a generic name would be fine here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I chose to be more generic there. The filter can be used for any type of widget, not just BU widgets.

* develop: (1262 commits)
  Migrate Open PR #407 from 1x-develop Security Audit June 2018 (#106)
  Migrate Open PR #418 from 1x-develop fix deprecated GF call (#107)
  PHP Unit Testing Updates (#97)
  Run grunt build to complete version bump to 2.1.9
  Bump version to 2.1.9 and update CHANGELOG
  Fix CHANGELOG nested ul indendation
  Fix multiple Code Climate issues on CHANGELOG
  Update CHANGELOG with bugfix notes
  Update CHANGELOG to use level 2 headings
  Bugfix: Prevent empty post title from rendering
  Bugfix: Fix typo in name of customizer filter
  version bump
  add unit tests for r_page_title_class
  add $display parameter to function update templates to use parameter
  Add filter to filter the page title class
  removing stray junk
  updating tests
  restoring code to old working version
  Add the global favicon back for non-BU domains
  version bump
  ...

# Conflicts:
#	CHANGELOG.md
* develop:
  bower cleanup (#121)
  version bump for release (#119)
  Move Foundation from bower to npm (#111)
@ashleykolodziej
Copy link
Contributor

ashleykolodziej commented Sep 25, 2020

I've tested this and updated the branch with the latest develop, but I think something is no longer working. Here is my test site:

http://id-ashley.cms-devl.bu.edu/kitchensink/

I added an empty BU Text widget to both the footbar and alternate footbar. I can see the incorrect widget count. On uploading this branch, the problem does not resolve. The homepage should show a count of 2, and any other page in the site should show a count of 9. I'm seeing 3 and 10 instead.

There weren't any merge conflicts, so I'm a little stumped. This is probably easy to fix, but needs a set of developer eyes to keep moving.

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

Successfully merging this pull request may close these issues.

Sidebars w/ empty BU Text/Link Widgets Produce Wrong Class Counts
3 participants