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

[plg_user_profile] Add RTL check to ToS field #20894

Merged
merged 2 commits into from
Jul 7, 2018
Merged

[plg_user_profile] Add RTL check to ToS field #20894

merged 2 commits into from
Jul 7, 2018

Conversation

SharkyKZ
Copy link
Contributor

@SharkyKZ SharkyKZ commented Jun 27, 2018

Pull Request for Issue # .

Summary of Changes

This adds RTL check to ToS field for popover positioning. Missed it in #20412.

Testing Instructions

Install and enable an RTL language. Enable the plugin and ToS field. Go to registration page and check that ToS field popover is positioned the same way as other popovers.

Expected result

Popover positioned correctly.

Actual result

Popover not positioned correctly.

Documentation Changes Required

No.

@infograf768
Copy link
Member

OK on review.

@Quy
Copy link
Contributor

Quy commented Jun 27, 2018

I have tested this item ✅ successfully on b22e63b


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

@ghost
Copy link

ghost commented Jul 2, 2018

@infograf768 can you please mark your Test as successfully on Issue Tracker?

@infograf768
Copy link
Member

infograf768 commented Jul 3, 2018

@SharkyKZ
while you are there, could you correct the alignment of the Agree/No radio buttons?
These small changes would work for both LTR and RTL:

		$css = '#jform_profile_tos {width: 18em; margin: 0 !important; padding: 0 2px !important;}
				#jform_profile_tos input {margin: 4px 5px 0 0 !important; width: 10px !important;}
				#jform_profile_tos label {margin: 0 20px 0 0 !important; width: auto;}
				';

After adding this patch I get here:
LTR
screen shot 2018-07-03 at 16 11 49

RTL (faked with French)
screen shot 2018-07-03 at 16 12 18

Before (RTL as it is much worse there):
screen shot 2018-07-03 at 16 13 43

@SharkyKZ
Copy link
Contributor Author

SharkyKZ commented Jul 3, 2018

@infograf768 thanks for the tip. Removing CSS completely. No sense to have it here.

@infograf768
Copy link
Member

Removing CSS completely. No sense to have it here.

Right, did not even test that before proposing the possible solution. 👍

@infograf768
Copy link
Member

I have tested this item ✅ successfully on 20eeef5


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

1 similar comment
@Quy
Copy link
Contributor

Quy commented Jul 3, 2018

I have tested this item ✅ successfully on 20eeef5


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

@Quy
Copy link
Contributor

Quy commented Jul 3, 2018

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 3, 2018
@mbabker mbabker added this to the Joomla 3.8.11 milestone Jul 7, 2018
@mbabker mbabker merged commit 9007811 into joomla:staging Jul 7, 2018
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jul 7, 2018
@SharkyKZ SharkyKZ deleted the plg_user_profile_rtl branch July 7, 2018 16:34
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

5 participants