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

1739 widget titles #1822

Closed
wants to merge 3 commits into from
Closed

Conversation

seanchayes
Copy link

Changes

This pull request makes the following changes:

The title field is added to the widget in admin for both the Facebook and Twitter widgets and when not empty the title is displayed. If a corresponding title link is entered into the widget this is used as the title link.

I did add escaping to the title text in the largo_add_link_to_widget_title function but understand that affects all the widgets. So, happy to help test or remove as appropriate. I tested the Taxonomy and Tax list widgets with no adverse affects.

Apologies for the occasional whitespace change.

Why

This PR applies consistent behavior for this widget
For #1739 and #1740

Testing/Questions

Features that this PR affects:

  • Largo Facebook widget
  • Largo Twitter widget
  • Widgets that use largo_add_link_to_widget_title callback on widget_title

Questions that need to be answered before merging:

  • Is this PR targeting the correct branch in this repository?

Steps to test this PR:

  1. Add widget to sidebar location
  2. Fill in title field in widget options and save
  3. Title text is in the markup if entered into title field
  4. Title text and title link url in the markup if both are filled in the widget options

Additional information

INN Member/Labs Client requesting: (if applicable)

  • Contributor has read INN's GitHub code of conduct
  • Contributor would like to be mentioned in the release notes as: (fill in this blank)
  • Contributor agrees to the license terms of this repository.

@benlk
Copy link
Collaborator

benlk commented Dec 20, 2019

Apologies for the occasional whitespace change.

These whitespace changes have been fine, as far as I see. There are a number of changes I'm making to this PR, though, to prevent some unset variables from causing problems, but you'll still get credit for this PR.

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.

2 participants