? ? Success

User tests: Successful: Unsuccessful:

avatar Quy
Quy
2 Oct 2020

Pull Request for Issue #29742.

Testing Instructions

Edit the login module
Set Display Labels = text
Go to front end

Actual result BEFORE applying this Pull Request

29742-before

Expected result AFTER applying this Pull Request

29742-after

avatar Quy Quy - open - 2 Oct 2020
avatar Quy Quy - change - 2 Oct 2020
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 2 Oct 2020
Category Modules Front End
avatar Quy Quy - change - 2 Oct 2020
Labels Added: ?
avatar brianteeman
brianteeman - comment - 2 Oct 2020

using title to display help text is not only ugly but its not accessible

avatar particthistle particthistle - test_item - 3 Oct 2020 - Tested successfully
avatar particthistle
particthistle - comment - 3 Oct 2020

I have tested this item successfully on f3b42c1

Layout issue that the PR is designed to resolve is corrected by the PR.

That said, looking at the code, I agree with @brianteeman, and the entire Login Form module is missing accessibility attributes for all the fields, so that's something for a different PR.

Additionally, I noticed for the first time today the "Web Authentication" button after turning on Two Factor Authentication to test this PR. Like 2FA, should this also not be turned on by default when Joomla 4 is installed? If it shouldn't, another PR.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30897.
avatar chmst
chmst - comment - 3 Oct 2020

I agree with @brianteeman that the title attribute is contra a11y. You can add a tooltip instead.

avatar adj9 adj9 - test_item - 3 Oct 2020 - Tested successfully
avatar adj9
adj9 - comment - 3 Oct 2020

I have tested this item successfully on f3b42c1

Done :)
OK!


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

avatar Scrabble96
Scrabble96 - comment - 4 Oct 2020

You can add a tooltip instead.

Tooltips don't show on iPad Safari and probably not on Android devices not using a mouse, either.

avatar ChrisHoefliger
ChrisHoefliger - comment - 17 Oct 2020

Tested successfully

avatar richard67
richard67 - comment - 17 Oct 2020

I agree with @brianteeman that the title attribute is contra a11y. You can add a tooltip instead.

@brianteeman @chmst Does it mean we should not set this PR to RTC? Or does it mean it should be changed with another, future PR?

avatar gostn
gostn - comment - 1 Dec 2020

who care on drafts having two successfully test?

avatar richard67
richard67 - comment - 1 Dec 2020

@gostn It's the author who decides if a PR is draft or not.

avatar gostn
gostn - comment - 1 Dec 2020

@Quy what now, test are wastet time?

avatar HLeithner
HLeithner - comment - 1 Dec 2020

Quy what now, test are wastet time?

@gostn claim down, PRs may involve over time and if Quy founds something that's not right or have to be changed because of other PRs are merged in the meantime it's good that the PR opener sets the PR to draft.

avatar gostn
gostn - comment - 1 Dec 2020

i dont make pressure, but at issutracker i list pr having 2 successfully test but not rtc – draft isnt shown in this list:

Screen Shot 2020-12-01 at 11 43 20

avatar Quy
Quy - comment - 1 Dec 2020

Sorry I don't have much time these days to contribute.

@gostn Accessibility to be fixed per Brian #30897 (comment)
See joomla-extensions/patchtester#286 regarding Draft status.

avatar infograf768
infograf768 - comment - 4 Dec 2020

conflicts

avatar infograf768
infograf768 - comment - 4 Dec 2020

If I choose icons instead of display text (after correcting conflicts), I get
Screen Shot 2020-12-04 at 09 41 29

Is that intended?

avatar Quy Quy - change - 4 Dec 2020
Labels Added: Conflicting Files
avatar ashvini77 ashvini77 - test_item - 5 Dec 2020 - Tested successfully
avatar ashvini77
ashvini77 - comment - 5 Dec 2020

I have tested this item successfully on 13dbfa6


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

avatar Scrabble96
Scrabble96 - comment - 5 Dec 2020

When the Cassiopeia updates were proposed in PR #31520 there was a comment (I can't find it now) suggesting that the checkbox size (next to 'remember me') was enlarged. I suggested the following code and now also include the RTL version. Perhaps this could be included with this PR if suitable?

/* ltr */
.com-users-login fieldset .control-label { margin-left: 2rem; }
.com-users-login fieldset input[type="checkbox"].inputbox { position: relative; transform: scale(1.5); top: -2rem; }

/* rtl */
.com-users-login fieldset .control-label { margin-right: 2rem; }

avatar gostn gostn - test_item - 6 Dec 2020 - Tested successfully
avatar gostn
gostn - comment - 6 Dec 2020

I have tested this item successfully on 13dbfa6


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

avatar Quy Quy - change - 6 Dec 2020
Status Pending Ready to Commit
avatar Quy
Quy - comment - 6 Dec 2020

RTC


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

avatar Quy
Quy - comment - 6 Dec 2020

@Scrabble96 Thank you for the code. It should be done in a separate PR since it is not related.

This PR fixes the alignment only. Let's fix the ay11 issue in a separate PR.

avatar drmenzelit drmenzelit - close - 6 Dec 2020
avatar drmenzelit drmenzelit - merge - 6 Dec 2020
avatar drmenzelit drmenzelit - change - 6 Dec 2020
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2020-12-06 19:20:27
Closed_By drmenzelit
Labels Added: ?
Removed: Conflicting Files
avatar drmenzelit
drmenzelit - comment - 6 Dec 2020

Thanks

Add a Comment

Login with GitHub to post a comment