Skip to content

[4.0] Fix toolbar collapse in mobile view#30131

Merged
infograf768 merged 8 commits intojoomla:4.0-devfrom
Quy:27958-toolbar
Sep 19, 2020
Merged

[4.0] Fix toolbar collapse in mobile view#30131
infograf768 merged 8 commits intojoomla:4.0-devfrom
Quy:27958-toolbar

Conversation

@Quy
Copy link
Contributor

@Quy Quy commented Jul 17, 2020

Fixes #27958.

Summary of Changes

Use the correct class to fix toolbar collapse in mobile view.

Testing Instructions

Apply PR.
Run npm i or install the package installer: https://ci.joomla.org/artifacts/joomla/joomla-cms/4.0-dev/30131/downloads/35475/
Resize browser's viewport.
Go to Global Configuration.
Go to any list views.

Actual result BEFORE applying this Pull Request

The toolbar toggle button requires 2 clicks to close. The first click attempts to open the already open toolbar.

Expected result AFTER applying this Pull Request

Toolbar collapsed by default.

@richard67
Copy link
Member

I have tested this item ✅ successfully on 91ff85b

PR works as decribed.

But there is another issue with the toolbar collapse not related to this PR, i.e. it is there with and without this PR.

When resizing the browser window, the toolbar collapsing and the gears button display doesn't happen at the same time. Especially when enlarging the window, first the gears button disappears, but for making the tool bar appear it needs to further enlarge the window, i.e. there is a small diplay width range in with neither the gears button to expand or collapse the toolbar nor the expanded toolbar are shown.

@Quy I guess that's a css problem. Do you think you can fix that, too? And do you know if there is an issue for that?


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

@Quy
Copy link
Contributor Author

Quy commented Jul 19, 2020

I am not at computer to test, but #30132 should have fixed the issue with the collapsed toolbar.

@richard67
Copy link
Member

Ahh, so both PRs together should do the job.

@richard67
Copy link
Member

@Quy No, the issue persists when I apply the changes of both PRs (this one here and #30132 ).

For display widths of 768 px or larger, the tool bar is shown, all fine.
j4-toolbar-w768

For display widths between 576 and 767 px, the toolbar is not shown, but also not the button for it.

Width 767 px:
j4-toolbar-w767

Width 576 px:
j4-toolbar-w576

For display widths of 757 px or smaller, the button is shown:
j4-toolbar-w575

@@ -211,11 +211,11 @@
}

if (smallLandscape.matches) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you see if changing to if (small.matches) { fixes the issue?

Copy link
Member

Choose a reason for hiding this comment

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

@Quy This fixes the problem which I've mentioned with the toolbar and its gears button, but it creates the same problem with the left side menu and its burger button, i.e. when resizing from small to a bit larger, the burger button disappears, but the (collapsed) menu doesn't appear. I think this was ok before the changes. It seems to me the 2 buttons are handled with the same css class but the toolbar and the menu have different sizes when they get collapsed or hidden.

@Quy Quy marked this pull request as draft July 23, 2020 00:01
@particthistle
Copy link
Member

particthistle commented Aug 1, 2020

Went to test this but the link to the PR files: https://ci.joomla.org/artifacts/joomla/joomla-cms/4.0-dev/30131/downloads/34008/ Gives File not found on all the downloads :(

Also when I went to apply the PR there was nothing able to be applied.

Modified per BFH discussion to indicate the failed test was not an actual test.

@richard67
Copy link
Member

richard67 commented Aug 1, 2020

Gives File not found on all the downloads :(

Also when I went to apply the PR there was nothing able to be applied.

@particthistle Both is not fault of this PR. The first one is caused by an erroneout cleanup script having cleaned up the wrong stuff, the 2nd thing is a patchtester thing.

So please change your test result back to "not tested" (can be done in the issue tracker by using the "Test this" button again).

It can never be the fault of a PR when something is wrong with inbfrastructure or test tools.

@richard67
Copy link
Member

@Quy I've just tested again and my comment above is still valid:

when resizing from small to a bit larger, the burger button disappears, but the (collapsed) menu doesn't appear.

@richard67
Copy link
Member

@Quy P.S.: But it seems to happen only after the first reducing and enlarging again the width. Ehen on the same page and again reducing and then enlarging width, it works.

@richard67
Copy link
Member

@Quy P.P.S.: If then going to another page and doing the same, it also happens only for the first time when reducing and then enlarging again the width. After that and a page reload, it works well on that page, until you have been on another page again.

@richard67
Copy link
Member

@Quy It seems the issue I've found is not related to this PR. Will continue to investigate and if necessary make new issue or PR. Could you fix the javascript cs error reported here https://ci.joomla.org/joomla/joomla-cms/35436/1/23 ? After that I can give this PR a good test.

@richard67
Copy link
Member

I have tested this item ✅ successfully on 22f88f0


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

@alikon
Copy link
Contributor

alikon commented Sep 19, 2020

I have tested this item ✅ successfully on 22f88f0


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

@alikon
Copy link
Contributor

alikon commented Sep 19, 2020

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Sep 19, 2020
@infograf768 infograf768 merged commit 9d523c6 into joomla:4.0-dev Sep 19, 2020
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Sep 19, 2020
@infograf768
Copy link
Member

Tks

@infograf768 infograf768 added this to the Joomla 4.0 milestone Sep 19, 2020
@Quy Quy deleted the 27958-toolbar branch September 19, 2020 18:56
sakiss pushed a commit to sakiss/joomla-cms that referenced this pull request Oct 16, 2020
* Fix toolbar collapse in mobile view

* Match toolbar collapse with cog icon

* Removed unused constant
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

NPM Resource Changed This Pull Request can't be tested by Patchtester

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

Comments