Skip to content

[4.0] Fix Actions dropdown display for Default#26815

Merged
wilsonge merged 2 commits intojoomla:4.0-devfrom
infograf768:4.0_dropdown_menu
Oct 25, 2019
Merged

[4.0] Fix Actions dropdown display for Default#26815
wilsonge merged 2 commits intojoomla:4.0-devfrom
infograf768:4.0_dropdown_menu

Conversation

@infograf768
Copy link
Member

Pull Request for Issue #26706

Summary of Changes

Taking off width for the icon

Testing Instructions

  • Menus > Main Menu
  • Select one menu item from the list and click on "Actions" button

Before patch the Default Action is broken

After patch

menudropdown

@Quy
Copy link
Contributor

Quy commented Oct 25, 2019

I have tested this item ✅ successfully on d34a08c


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

@1apweb
Copy link

1apweb commented Oct 25, 2019

I have tested this item 🔴 unsuccessfully on d34a08c

Applying the patch nothing changes!


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

@richard67
Copy link
Member

@1apweb You have to have npm installed and run npm to compile the scss. This is always the case is some scss or js is changed by a Pull Request.

You can find a description about that here: https://docs.joomla.org/J4.x:Setting_Up_Your_Local_Environment.

Please change your test result in the issue tracker to "I have not tested this item" or so, a negative test result is wrong here.

@infograf768 infograf768 added the NPM Resource Changed This Pull Request can't be tested by Patchtester label Oct 25, 2019
@1apweb
Copy link

1apweb commented Oct 25, 2019

I have not tested this item.


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

@ChristineWk
Copy link

ChristineWk commented Oct 25, 2019

I have tested this item ✅ successfully on fe4a724

I have same result as shown on screenshot. Threre is nothing broken. But I didn't hv to run npm ... btw I can't this :-)
tested with beta-1


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

@richard67
Copy link
Member

... But I didn't hv to run npm ... btw I can't this :-)
tested with beta-1

@ChristineWk You mean with beta-1 the patchtester 4.0-beta1? In this case and if you have CI integration switched on in the patchtester and are not affected by a bug which has been fixed after beta-1, then the patchtester has done the work for you which npm does (but in a very different way).

But if you mean with beta-1 the Joomla nightly build (correctly beta1-dev, that is the current CMS development version) then you should have had to use npm. Then I am surprised that you had a good test, and in my opinion the test should not be valid.

Could you report back which of the 2 possibilities described above is the right one?

@infograf768
Copy link
Member Author

@ChristineWk
the patch is towards the present status of 4.0-dev
Before patch:
https://www.youtube.com/watch?v=OTzCsbuAHIE

@richard67
Copy link
Member

I have tested this item ✅ successfully on fe4a724

Hints for other testers:

  1. It might be necessary to forced reload the page with the menu items list in your browser or clear browser cache after having applied the patch and run npm i.

  2. While npm runs, don't fall asleep .. joking.


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

@Quy Quy removed the NPM Resource Changed This Pull Request can't be tested by Patchtester label Oct 25, 2019
@Quy
Copy link
Contributor

Quy commented Oct 25, 2019

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Oct 25, 2019
@ChristineWk
Copy link

@richard67

OK, wait 1 minute or more - I hv to sort it :-) Just saw your questions now.


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

@richard67
Copy link
Member

@ChristineWk Check also infograf's video about how it is broken without the PR.

@ChristineWk
Copy link

Here are the results :-)
Hmmm. Checked infografs video - it's not broken - without PR!

  • Full fresh nightly build on oct 20th installed
  • … ci.joomla.org:44 was: off!
  • Joomla! Patch Tester | Administrator | Component | 4.0.0-beta2 | 17-October-2019
  • Site | File | No Problems | 4.0.0-2019-10-06 | ‎4.0.0-beta1-dev
    pr26815

@richard67
Copy link
Member

richard67 commented Oct 25, 2019

@ChristineWk Here it was broken like shown in the video. I tested on current 4.0-dev, the issue handled by this PR is from 6 days ago, that would be October 19, i.e. before your nightly build from 20th, right? ... stange, very strange. If the issue would have been created after the nightly build it would be clear to me.

@Quy
Copy link
Contributor

Quy commented Oct 25, 2019

@ChristineWk Please use today's nightly build. It will have PR #26757 merged 2 days ago that this PR will fix.

@richard67
Copy link
Member

@ChristineWk Ah, I just see: The issue was fixed with PR #26056 6 days ago. Then came your nightly build, but later that PR was reverted for whatever reason, maybe it broke something else. That explains why you can't reproduce the issue. With a nightly build of today you could.

@richard67
Copy link
Member

@Quy We posted almost simultaneously, funny.

@ChristineWk
Copy link

OK - it was an unsuccessful success :-)
@richard67 btw: can I use via custom URL the update nightly build? Because ... remember my previous issue, where my alpha-12-dev has been broken ... now I hv (see above) some day old fresh version.

@richard67
Copy link
Member

@ChristineWk You mean update your nightly build from october 20 to nightly build of today?

Officially this is not supported.

Inofficially I would say you can do that if between these 2 nightly build there is not any difference in joomla.sql files and in those countless files in administrator/components/com_admin/sql/updates/XXX, with XXX being your database type. But as soon as there are changes in those sql files: Keep hands off!!! Better do a new install.

Btw., if you need a good tool to quickly compare if files in 2 folders are different, I recommend BeyondCompare ;-)

@richard67
Copy link
Member

@ChristineWk As soon as J4 Beta-1 will be released, it will be supported to update from beta-x to beta-y (with y > x of course, but not necessarily y = x+1, it can also be any later beta) or from any beta to RC-x and so on. Only during Alpha status it is not supported yet because otherwise it would have almost been impossible for us to work on bringing our database schema into 21st century with recent works on nulldates and other things.

@wilsonge wilsonge merged commit 0cd0af0 into joomla:4.0-dev Oct 25, 2019
@wilsonge
Copy link
Contributor

Thanks!

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Oct 25, 2019
@wilsonge wilsonge added this to the Joomla 4.0 milestone Oct 25, 2019
@ChristineWk
Copy link

@richard67

aha, oh oh - hv to study again - your 2nd comment is like algebra :-)
Thanks for your information! rgds.

@infograf768 infograf768 deleted the 4.0_dropdown_menu branch October 26, 2019 05:01
@infograf768
Copy link
Member Author

Note to whoever is concerned: this Actions Dropdown is a pain to adapt to RTL...

@Stuartemk
Copy link

What is the meaning of two icons?

For what two?

For example the filter options also performs an action, but it does not have: fa fa-ellipsis-h

Thank you.

@C-Lodder
Copy link
Member

Just an FYI, the width wasn't being applied to the icon- classes because they were inline elements, whereas the fa classes are inline-block elements

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.

9 participants