? Success

User tests: Successful: Unsuccessful:

avatar asika32764
asika32764
21 Feb 2014

Test Instructions

Please go to option=com_cache page, you will notice that we have to click the checkbox to check every item, there is no use to click item title.

p-2014-06-14-4

Then we checkout to this PR, now just click item title, the checkbox will be checked.

p-2014-06-14-5

This is an UX enhancement update.

Changed pages:

  • Joomla Installer default language page
  • Admin Language Manager: Site & Admin list
  • Admin Installer: Install language
  • Admin Installer: Manage
  • Admin Installer: Discover
  • Admin Installer: Update

140221-0002
140221-0003
140221-0004
140221-0005
140221-0006
140221-0007

Tracker: http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=33321&start=0

avatar asika32764 asika32764 - open - 21 Feb 2014
avatar infograf768
infograf768 - comment - 21 Feb 2014

Great! Commented on tracker.

avatar asika32764
asika32764 - comment - 24 Feb 2014

Some update based on discussion on Tracker

  • Global Check-in
  • Cache Clear
  • SmartSearch Index
avatar asika32764
asika32764 - comment - 14 Jun 2014

Is there any news about this PR since people has already tested it?

avatar roland-d
roland-d - comment - 14 Jun 2014

@asika32764 On the tracker item Brian states he doesn't see any changes. So I tested this also and I don't see any changes either. For example when I look at the page administrator/index.php?option=com_cache I notice the table to jump a little but no noticeable difference. In the source I do see the difference, as there is a label tab. What is the use for end-users if they don't see a difference?

When I apply the patch I get a codestyle warning:
:89: space before tab in indent.

warning: 1 line adds whitespace errors.

It will be useful if you can add test instructions, I really don't know what to look for other than may be the label.

avatar asika32764
asika32764 - comment - 14 Jun 2014

@roland-d Thank you for reply. I added the test step at top of this PR.

avatar roland-d
roland-d - comment - 14 Jun 2014

@asika32764 I saw the test instructions at the top but I don't see any change in my browser except for looking at the source code. So I am unclear as to what changes I should see. Can you explain that?

avatar asika32764
asika32764 - comment - 14 Jun 2014

OK, the HTML <label> is an element that direct to an input, and if mouse click this label, the corresponding input will be focused.

Sometimes we write the form like input box, checkboxes or radio, this element are small and hard to select or click.

Label will help us easier to focus on these inputs. For this PR, before label adding, we have to click the checkbox to check it, but after label added, we can just click the title, then the checkbox will be checked.

So we are not able to see any change about the layout, but just use mouse to click table list item, the checkbox will be auto checked.

avatar roland-d
roland-d - comment - 17 Jun 2014

@asika32764 The test is all good, there is only 1 small problem with the PR. I get this notice:
:89: space before tab in indent.

warning: 1 line adds whitespace errors.

Please fix that and then we can move this forward. Thanks.

avatar asika32764
asika32764 - comment - 18 Jun 2014

Thanks, could you tell me which file?

I don't see any 89 line in changes diff of this PR

avatar roland-d
roland-d - comment - 18 Jun 2014

Here is the full set I see when I apply and reverse this PR:

D:\wamp\www\joomla-cms>curl https://github.com/joomla/joomla-cms/pull/3154.diff
| git apply
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 10164 100 10164 0 0 16262 0 --:--:-- --:--:-- --:--:-- 20328
:89: space before tab in indent.

warning: 1 line adds whitespace errors.

D:\wamp\www\joomla-cms>curl https://github.com/joomla/joomla-cms/pull/3154.diff
| git apply -R
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 10164 100 10164 0 0 18085 0 --:--:-- --:--:-- --:--:-- 22437
:88: space before tab in indent.
"name, $item->description, 0); ?>"><?php ec
ho $item->name; ?>
warning: 1 line adds whitespace errors.

HTH

avatar asika32764
asika32764 - comment - 18 Jun 2014

Sorry but this information still hard to find the file. I think this warning is not caused by my code, maybe it has already exists from the past. I opened every files I modified and my PHPStorm didn't warning me about Code Style fail.

avatar roland-d
roland-d - comment - 18 Jun 2014

@asika32764 I found the file causing the problem, administrator/components/com_installer/views/discover/tmpl/default.php that is the culprit. This file has more CS issues, the line you changed may just triggered it. Since you use PhpStorm it might be an idea to go to Edit -> Convert Indents -> To Tabs. Then commit the file again and I can check if the notice is gone.

avatar asika32764
asika32764 - comment - 18 Jun 2014

Thanks for helping, I'm done.

avatar roland-d
roland-d - comment - 18 Jun 2014

All looks good now, I get no errors when applying the patch. I do see the errors on reverting the patch but they must come from the original file and should be fixed when your PR is committed.

I have set the Feature Request to Accepted, hopefully that is correct ;)

avatar infograf768
infograf768 - comment - 19 Jun 2014


I have set the Feature Request to Accepted, hopefully that is correct ;)

Not really... ☺

Only PLT can do that

avatar roland-d
roland-d - comment - 19 Jun 2014

So we leave the status at whatever status it is?

avatar Bakual
Bakual - comment - 19 Jun 2014

Set it to Ready for review as soon as there are enough tests. It's the equivalent to Ready to commit.

avatar roland-d
roland-d - comment - 19 Jun 2014

@Bakual Thanks, and updated JC tracker.

avatar Bakual
Bakual - comment - 11 Jul 2014

Merged into 3.4-dev. Thanks!

avatar Bakual Bakual - change - 11 Jul 2014
Status New Closed
Closed_Date 0000-00-00 00:00:00 2014-07-11 06:50:37
avatar Bakual Bakual - close - 11 Jul 2014

Add a Comment

Login with GitHub to post a comment