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

Add layouts to plg_user_terms #23787

Merged
merged 4 commits into from Feb 19, 2019
Merged

Add layouts to plg_user_terms #23787

merged 4 commits into from Feb 19, 2019

Conversation

Ruud68
Copy link
Contributor

@Ruud68 Ruud68 commented Feb 7, 2019

Pull Request for Issue # n/a.

Summary of Changes

the layouts for plg_user_terms (label and termsnote) are hardcoded into the field and cannot be overwritten by a template override
This PR adds layouts for both the label and for the termsnote (message). these layouts can be overwritten via template overrides.

Testing Instructions

  1. enable the terms userm plugin
  2. set a joomla article as terms article in the plugin
    on the front-end user registration page the terms box should show

Apply this PR

[test] on the front-end user regsitration page the terms box should be exactly the same as without this PR.

  1. create a template override via the template (e.g. protostar):
    -> tab Create Overrides
    -> column Layouts
    -> select plugins > user

Edit the created override file: ./templates/protostar/html/layouts/plugins/user/terms/message.php
4. change the class from alert-info to alert-warning

[Result 1] refresh on the front-end the new user registration form

Edit the created override file: ./templates/protostar/html/layouts/plugins/user/terms/label.php
5. change <span class="star">&#160;*</span></label> to <span class="star">&copy;*</span></label>

[Result 2] refresh on the front-end the new user registration form

New result

image

Actual result

Make sure that after applying the PR everything works as before.

Documentation Changes Required


$attribs = array();
$attribs['class'] = 'modal';
$attribs['rel'] = '{handler: \'iframe\', size: {x:800, y:500}}';
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please move this to use Bootstrap Modal instead of the Mootools?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, my aim is to do one change a time. The aim of this PR is to add layout overrides for this plugin, just as I did for the plg_system_privacyconsent, not to change the logic.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's 2019, we shouldn't use Mootools.-

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what is the code change needed for switching from mootools to bootstrap modal?

Copy link
Contributor

Choose a reason for hiding this comment

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

See #22926 for an example.

Converting from MooTools to Bootstrap is a change for another PR. Please do not include it 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.

Thanks @mbabker and agree

@tecpromotion
Copy link
Contributor

I have tested this item ✅ successfully on c56805f

I have tested this item successfully on 3.9.2.


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

@Ruud68
Copy link
Contributor Author

Ruud68 commented Feb 8, 2019

Thanks for testing @tecpromotion :) Appreciate it!


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

layouts/plugins/user/terms/label.php Outdated Show resolved Hide resolved
@viocassel
Copy link
Contributor

I have tested this item ✅ successfully on 13a7114


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

@Ruud68
Copy link
Contributor Author

Ruud68 commented Feb 9, 2019

Thanks for testing @viocassel

@Ruud68
Copy link
Contributor Author

Ruud68 commented Feb 14, 2019

@Quy I see two successful tests, only one is counted? Do you know why and what is needed for this to go RTC?


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

@Bakual
Copy link
Contributor

Bakual commented Feb 14, 2019

When a new commit is made after a test, you need to retest it. The old tests don't count anymore.

@Ruud68
Copy link
Contributor Author

Ruud68 commented Feb 14, 2019

@Bakual which is strange, because the one counted as successful is the one before the last change? anyway @viocassel and @tecpromotion just to make sure, can you both (re) test #please :)

@Bakual
Copy link
Contributor

Bakual commented Feb 14, 2019

It shows the one from Vio as successful one. The first one from Stefan is discarded since you did two commits afterwards.
image
So @tecpromotion or someone else has to test (again).

@tecpromotion
Copy link
Contributor

I have tested this item ✅ successfully on 7ff2f05

I have tested this item successfully on 3.9.3.


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

@viocassel
Copy link
Contributor

I have tested this item ✅ successfully on 7ff2f05


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

@HLeithner HLeithner merged commit 49eacdc into joomla:staging Feb 19, 2019
@HLeithner
Copy link
Member

thx

@HLeithner HLeithner added this to the Joomla 3.9.4 milestone Feb 19, 2019
@Ruud68 Ruud68 deleted the plgusertermslayouts branch February 22, 2019 12:55
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.

None yet

9 participants