[4.0] Fix RTL toolbar

[brianteeman]

see screenshots for changes

If we wait for an expert to fix it then it will never happen

Before

After

[infograf768]

Will test tomorrow. For the moment just a general remark.
It looks that the system searchtools.css is loaded differently in ltr and rtl vs the specific atum searchtools.scss which is included into atum template.css.
I discovered that when trying to solve the rtl float depending on browser window size for the client field In the installed language manager. It is extremely confusing and it may be that we have to fully understand what is going on here before touching again searchtools.

[infograf768]

@dgrammatiko
Can you help solving the loading order of rtl template css and system searchtools.css?
It also looks that it would be much more simple to do all the searchtools css only in the template block scss and get rid of the system one as it is extremely confusing to deal with both... We never know which one takes precedence over the other, etc.

[infograf768]

This ltr margin one should also be deleted. Keep display: inline-block;

[dgrammatiko]

@infograf768 what exactly is broken?
About returning back to monolithic css: you can revert my pr...

[infograf768]

@dgrammatiko
I did not say return to monolithic css. Using blocks _searchtools.scss is fine
The problem here is that we have 2 places where the searchtools css are fetched from

1. _searchtools.scss from Atum blocks creates css in the template.
2. A general searchtools.css in media/system/css/

It happens, as stated above, that in rtl, parts of the media/system/css/ searchtools.css is loaded after the template searchtools classes for some cases, creating the issue
Do we really need that file is the question? Or at least can you solve the issue?

LTR

RTL

The css in the template is created by the scss

  .js-stools-container-selector,
  .js-stools-container-selector-first {
     @include media-breakpoint-down(md) {
        float:none;
        width:100%;
        margin-right:0.5rem;
    }

Which should override the searchtools.css as it has not specific direction
It does for ltr, not for rtl.

In the searchtools.css in system we have

.js-stools .js-stools-container-selector {
	float: left;
	margin-right: auto;
}

and further

html[dir=rtl] .js-stools .js-stools-container-selector {
	float: right;
	margin-left: auto;
	margin-right: 0;
}

The only solution I found was again to add a css in the system one using html[dir=rtl] and @media etc.
That is really bothering as it really look as a hack.

[dgrammatiko]

@infograf768 on my mobile, so not much I can do right now but I just saw that there is a _searchtools.scss in the sass/blocks. This is utterly wrong! Rename it to searchtools.scss and move it in the parent dir next to template.scss. Also add an @import path/to/system/searchtools.css at the top. That should fix it

[infograf768]

@dgrammatiko
Sorry to say, but I tried as you proposed and it does not work.
First I had to also modify template.scss to use
@import "searchtools"; insteaf of @import "blocks/searchtools"; as the file is moved.

The path I added is @import "../../../../media/system/css/searchtools.css";
It does not look like working. Can one really import a .css file into a .scss file?
I tried it in template.scss as well as in the searchtools.scss without luck.
only result is that searchtools.css is loaded twice.

Also remains in joomla.asset.json this weird line

      "css": [
        "system/searchtools.css"
      ]

The whole thing is too confusing for me.

I also still don't understand the presence of 2 different css files for searchtools, i.e. as I guess media/system/css/searchtools.css has to be loaded whatever the admin template, what should it exactly contain that can be customised/overriden in the atum searchtools.scss ?

[infograf768]

hmm
We should not use the .css suffix when importing
Should be
@import "../../../../media/system/css/searchtools";

Also remains to solve the issue in rtl, i.e. decide what should exactly contain media/system/css/searchtools.css vs the template-rtl.css

[dgrammatiko]

First I had to also modify template.scss to use @import "searchtools"; insteaf of @import "blocks/searchtools"; as the file is moved.

You don't have to do that! What I proposed should just override the default loaded css, which is the correct way. What you are doing right now is loading the module css and then overriding it in the monolithic template.css. That's not how you suppose to write css in 2020. Happy new year...

Can one really import a .css file into a .scss file?
Yes, just remove the extension, IIRC, but you solved that already.

Just leave it as a separate file in the scss folder, remove the import from the template, and maybe add the override in the assets.JSON, you'll have to check how this new system works

[infograf768]

I am lost...

[infograf768]

@dgrammatiko

That's not how you suppose to write css in 2020. Happy new year...

Please do the PR. I am failing miserably, and it is not because I try to code in a pre-2020 way. It is because I mistake following your suggestions, or that I don't understand the details of them, or because something is wrong in your suggestions.

[dgrammatiko]

@infograf768 well, it's a bit more complicated and I should have mentioned that #25775 and #27268 are prerequisites here as the asset manager in its current form is a bit broken for these kind of operations. Ask someone with merging rights to push those 2. Then my comments will suddenly make much more sense

[infograf768]

I do have merging rights but I don’t usually merge something I don’t understand. It is the case for these.

[Fedik]

if you need to drop/overide of use system/css/searchtools, you can override the asset (searchtools asset) definition for active template,
add to administrator/templates/atum/joomla.asset.json

{
    "name": "searchtools",
    "dependencies": [
      "core"
    ],
    "js": [
      "media/system/js/searchtools.min.js"
    ],
    "css": []
},

note "css": [] is empty, this will remove default searchtools.css from being loaded

[dgrammatiko]

note "css": [] is empty, this will remove default searchtools.css from being loaded

I think this needs to point to the templates/atum/css/searchtools.min.css, we are not supposed to remove the css but rather use the css that is tightly coupled to this particular template. The point is that this css file should only be present on the pages that actually have searchtools, I don't know if asset manager will take care of that, will it?

[Fedik]

for path templates/atum/css/searchtools.min.css it then:

{
    "name": "searchtools",
    "dependencies": [
      "core"
    ],
    "js": [
      "media/system/js/searchtools.min.js"
    ],
    "css": [
      "searchtools.min.css"
    ]
},

it equal for JHtml::_("stylesheet", "searchtools.min.css" .... );

[Fedik]

The point is that this css file should only be present on the pages that actually have searchtools

All in joomla.asset.json it just a definitions, it not loaded if you not call it to load somewhere in the code with $assetManager->useAsset('blabla').
So it will be used only when a searchtool layout call to $assetManager->useAsset('searchtools').

[infograf768]

At this time, we load for Atum both the blocks/searchtools.scss which goes into the template.css and therefore into template-rtl.css, AND media/system/css/searchtools.css

I still have no idea what should be loaded and what should be or not overriden.
Should the system one be overriden and in that case by what, AND if it should, what shall it contain?

We obviously can’t go on trying solving various display issues in rtl as well as ltr in the present situation.

Please make a PR and tell us where we should add/delete/modify.

[Fedik]

we load for Atum both the blocks/searchtools.scss which goes into the template.css and therefore into template-rtl.css, AND media/system/css/searchtools.css

so all styling for searchtools already done in the Atum template(-rtl).css ?
then we do not need to load core searchtools.css at all, then use my first suggestion, which disable core searchtools.css, with "css": []

upd:

Please make a PR

I will try, a bit later

[dgrammatiko]

so all styling for searchtools already done in the Atum template(-rtl).css ?
then we do not need to load core searchtools.css at all, then use my first suggestion, which disable core searchtools.css, with "css": []

Sorry, but this is WRONG! The template SHOULDN'T be bloated. Each part of the Joomla (form element, search tools, etc) has it's own css. We have to keep this modularity, packing a 1/2 MB css file is not good, but the worst part is that people copy what the core is doing and they blindly reproduce it in their sites. So, with that in mind, please keep things modular...

[Fedik]

yeah I know,
I have answered for current state

[infograf768]

Final thought, do you really need an extra .rtl.css ? I mean you could do etc.

It is easy to add rtl the way below in the blocks/scss/ files and not creating fully new cascading series which would make the template.css enormous.
Separating all blocks scss into their smaller .css and not grouping the classes in a unique file would not solve the issue at stake anyway.

#content {
  .menu-quicktask {
    position: absolute;

    [dir=ltr] & {
      right: 1.25rem;
    }

    [dir=rtl] & {
      left: 1.25rem;
    }
  }
}

or

.dropdown-header {
      color: $white;
      background-color: var(--atum-bg-dark);
      box-shadow: $atum-box-shadow;
      font-size: inherit;
      padding: 1rem 0.75rem;

      [dir=rtl] & {
        text-align: right;
      }
    }

template-rtl.css contains 257 lines of classes that would have to be overriden or just created in the various scss, mostly in _global.scss
Quite a refactoring job.

But this is not the main point here... and my question receives no answer.
I therefore repeat it:

This is why I am insisting, in case it is decided to load only the specific css for ATUM (separated or not), to know what should exactly contain the system one...

[dgrammatiko]

to know what should exactly contain the system one

I'm not the one designed or coded that thing so I cannot answer that. I think you should ping other people for that answer...

[Fedik]

yeah, the solution: or move all from searchtools.css to atum/css/searchtools.css or vice versa

I tried here 4.0-dev...Fedik:searchtools-move-to-tmpl
It just a "blind copy" of searchtools.css and blocs/_searchtools.css to atum/css/searchtools.css

But it really better to do by someone who know the template.

[dgrammatiko]

@Fedik that's a good starting point

[infograf768]

Will test tomorrow and try to modify the blind copy to something that makes sense by combining classes for ltr and rtl.
(If I understand well, we would definitely get rid of media/system/css/searchtools.css)

[infograf768]

@Fedik
here is a consolidation of the new searchtools.scss to replace the one you have in your branch.
Please make PR.
searchtools.scss.zip

Note1: @brianteeman
It includes what concerns searchtools (not toolbar) in this PR, as well as the modifications in #27388

Note2: There will still be changes to do in toolbar, both for ltr and ltr, specially for mobile.
Once @Fedik future PR is merged it will be easier to do the modifications

Note3: It looks that some classes may not be of use anymore in the new scss, but hard to find out as they are also defined in the js.

[Fedik]

@infograf768 there it is #27416
I have copied your searchtools.scss.zip file to it

It looks that some classes may not be of use anymore in the new scss, but hard to find out as they are also defined in the js.

they may be from j3, that almost not possible to find,
in theory can just remove them and see where it will explode 😃

[infograf768]

See #27416

@brianteeman
Please take off the searchtools part from this PR as it is included in #27416

[brianteeman]

done as requested

[dgrammatiko]

Since #27416 was merged I guess also this one should follow the same path, eg place the scss in the right folder, remove the _ and do the needed changes in the template.asset.json. Let's modularise instead of throwing everything in one file

PS the file that this code is overriding is:

joomla-cms/build/media_source/system/scss/joomla-toolbar-button.scss

Line 1 in 00c613e

joomla-toolbar-button .dropdown-item:focus,

[infograf768]

@dgrammatiko
We should do that in a specific pr and not only for that blocks/_toolbar.scss
For the moment I am merging this as we need it and, contrary to searchtools we have no confusion with another toolbar.css in media

[infograf768]

Tks

[brianteeman]

thanks