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

Enhance Content - Contact plugin #18258

Merged
merged 4 commits into from Jun 12, 2018
Merged

Enhance Content - Contact plugin #18258

merged 4 commits into from Jun 12, 2018

Conversation

@mattiaverga
Copy link
Contributor

@mattiaverga mattiaverga commented Oct 6, 2017

Summary of Changes

Currently Content - Contact plugin can only link the author name to the internal page of the contact associated to the author of the article.
This PR enhances the plugin with the ability to choose the webpage or the email of the associated contact as link to the author.
An option is added if the admin doesn't want to apply links to the real author when an alias name is used as article author.

Testing Instructions

Enable "Show author" and "Link author" in global article options, or just in one article; enable Content - Contact plugin; associate a contact to the user who created the article.

Expected result

When the article is displayed, the author name will be linked to the internal contact page (this is the default and it works like before).
You can change the type of link in the plugin configuration: "website" will link the author name to the website specified in the contact details (if any); "email" will open the default email client to email to the address provided in contact details.

Actual result

Only the link to the internal contact page is available.

@infograf768
Copy link
Member

@infograf768 infograf768 commented Oct 8, 2017

This works fine but needs some code improvements.
I modified your xml to fit

<?xml version="1.0" encoding="utf-8"?>
<extension version="3.2" type="plugin" group="content" method="upgrade">
	<name>plg_content_contact</name>
	<author>Joomla! Project</author>
	<creationDate>January 2014</creationDate>
	<copyright>Copyright (C) 2005 - 2017 Open Source Matters. All rights reserved.</copyright>
	<license>GNU General Public License version 2 or later; see LICENSE.txt</license>
	<authorEmail>admin@joomla.org</authorEmail>
	<authorUrl>www.joomla.org</authorUrl>
	<version>3.2.2</version>
	<description>PLG_CONTENT_CONTACT_XML_DESCRIPTION</description>
	<files>
		<filename plugin="contact">contact.php</filename>
	</files>
	<languages>
		<language tag="en-GB">en-GB.plg_content_contact.ini</language>
		<language tag="en-GB">en-GB.plg_content_contact.sys.ini</language>
	</languages>
	<config>
		<fields name="params">
			<fieldset name="basic">
				<field
					name="url"
					type="list"
					label="PLG_CONTENT_CONTACT_PARAM_URL_LABEL"
					description="PLG_CONTENT_CONTACT_PARAM_URL_DESCRIPTION"
					default="0"
				>
					<option value="0">PLG_CONTENT_CONTACT_PARAM_URL_0</option>
					<option value="1">PLG_CONTENT_CONTACT_PARAM_URL_1</option>
					<option value="2">PLG_CONTENT_CONTACT_PARAM_URL_2</option>
				</field>

				<field
					name="link_to_alias"
					type="radio"
					label="PLG_CONTENT_CONTACT_PARAM_ALIAS_LABEL"
					description="PLG_CONTENT_CONTACT_PARAM_ALIAS_DESCRIPTION"
					default="0"
					class="btn-group btn-group-yesno"
				>
					<option value="0">JNO</option>
					<option value="1">JYES</option>
				</field>
			</fieldset>
		</fields>
	</config>
</extension>

To get:

screen shot 2017-10-08 at 11 02 09

remark that we use JNO and JYES, therefore the strings

PLG_CONTENT_CONTACT_PARAM_YES="Yes"
PLG_CONTENT_CONTACT_PARAM_NO="No"

should be taken off the ini file.

Also the fields name do usually use lowercase characters.
I changed them to
url and link_to_alias
Which means they should be changed in code.

Also I would change
PLG_CONTENT_CONTACT_PARAM_URL_LABEL="URL to link to"
to maybe a simple
PLG_CONTENT_CONTACT_PARAM_URL_LABEL="Redirection"
as obviously an email is not an url :)

Also use lowercase for html tags in text strings, i.e. not <UL><LI> but <ul><li>

@brianteeman
Copy link
Member

@brianteeman brianteeman commented Oct 8, 2017

@infograf768 the > needs to be indented like this

					default="0"
					>
					<option value="0">PLG_CONTENT_CONTACT_PARAM_URL_0</option>
@mattiaverga
Copy link
Contributor Author

@mattiaverga mattiaverga commented Oct 8, 2017

Should the version number also be upgraded?

I have another doubt: in the xml the "extension version" is set to 3.2, but in php code the "protected $db;" code says it's available since 3.3...

@infograf768
Copy link
Member

@infograf768 infograf768 commented Oct 8, 2017

No need to touch the versions.

@infograf768
Copy link
Member

@infograf768 infograf768 commented Oct 8, 2017

I have tested this item successfully on 5693292

Works fine here. May need some tweaks to the values of the ini strings. @brianteeman


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18258.

@ghost
Copy link

@ghost ghost commented Oct 9, 2017

I have tested this item successfully on 5693292

Handy Improvement, thanks.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18258.

@ghost
Copy link

@ghost ghost commented Oct 9, 2017

RTC after two successful tests.

@joomla-cms-bot joomla-cms-bot added the RTC label Oct 9, 2017
@infograf768
Copy link
Member

@infograf768 infograf768 commented Nov 2, 2017

@mbabker Please decide a milestone for this.

@mbabker mbabker added this to the Joomla 3.9.0 milestone Nov 24, 2017
description="PLG_CONTENT_CONTACT_PARAM_URL_DESCRIPTION"
default="0"
>
<option value="0">PLG_CONTENT_CONTACT_PARAM_URL_0</option>

This comment has been minimized.

@mbabker

mbabker Jun 2, 2018
Contributor

Instead of 0/1/2 for the option values can we use text names so it's clear what each of these options are (so url/webpage/email).

@mattiaverga mattiaverga requested a review from brianteeman as a code owner Jun 3, 2018
Copy link
Member

@brianteeman brianteeman left a comment

approve string changes

@mbabker
mbabker approved these changes Jun 3, 2018
@mbabker mbabker modified the milestones: Joomla 3.10.0, Joomla 3.9.0 Jun 12, 2018
@mbabker mbabker changed the base branch from staging to 3.9-dev Jun 12, 2018
@mbabker mbabker merged commit 06e210c into joomla:3.9-dev Jun 12, 2018
3 of 4 checks passed
3 of 4 checks passed
continuous-integration/drone/pr the build failed
Details
Hound No violations found. Woof!
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@joomla-cms-bot joomla-cms-bot added PR-3.9-dev and removed RTC labels Jun 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.