-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[4.0] Searchtools mobile display #27443
Conversation
quickcheck - looks great |
[dir=rtl] & { | ||
margin-right: 0; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as the value is the same for LTR and RTL you should not need to have individual settings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree. There are other places to also simplify. let's wait for more small changes to modify.
} | ||
|
||
@include media-breakpoint-down(xs) { | ||
float: none !important; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already defined under media-breakpoint-down(sm)
|
||
@include media-breakpoint-down(xs) { | ||
float: none !important; | ||
width: 100%; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already defined under media-breakpoint-down(sm)
width: 100%; | ||
|
||
[dir=ltr] & { | ||
margin-right: 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already defined under media-breakpoint-down(sm)
} | ||
|
||
[dir=rtl] & { | ||
margin-right: 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already defined under media-breakpoint-down(sm)
} | ||
|
||
@include media-breakpoint-down(xs) { | ||
max-width: 100%; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already defined under media-breakpoint-down(sm)
margin-right: .5rem; | ||
|
||
[dir=rtl] & { | ||
margin-right: 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already defined under media-breakpoint-down(sm)
@@ -143,6 +201,15 @@ | |||
margin-right: 0; | |||
} | |||
} | |||
|
|||
@include media-breakpoint-down(xs) { | |||
max-width: 100%; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already defined under media-breakpoint-down(sm)
margin-right: .5rem; | ||
|
||
[dir=rtl] & { | ||
margin-right: 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already defined under media-breakpoint-down(sm)
It works but it needs cleaning up to prevent repetition |
Alas I had already merged 8b25305 Therefore your proposals were hard to find out. Please look again and test. If you propose some more changes, please test them before, following my instructions. |
It looks like you got them all. On a side note, there is a couple of ways of improving this...
|
@ciar4n We have 3 solutions:
|
On a single column layout there is no reason why they should. The frontend template CSS is mobile first, which is why I suggested it here. |
Then please make patch. |
I have tested this item ✅ successfully on 118dc6f This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/27443. |
@ciar4n @brianteeman |
I have tested this item ✅ successfully on 118dc6f This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/27443. |
RTC This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/27443. |
Thanks |
Follow-up on #27416
Summary of Changes
Take off from _toolbar.scss
.js-stools-container-selector
which interfere with searchtools.cssCorrect margins in mobile view as the situation is different when we have
.js-stools-container-selector
(1 field) or.js-stools-container-selector-first
(2 fields).Testing Instructions
Install multiple languages including Persian (fa-IR)
Display in Mobile view the Installed language manager and also the Menu Items manager.
These contains fields separated from the main searchtools (site/admin for Installed languages and site/admin // menu for menu items.
Patch and use npm
Before patch
LTR
RTL
After patch
LTR
RTL
@Fedik @brianteeman