NPM Resource Changed ? ? Pending

User tests: Successful: Unsuccessful:

avatar drmenzelit
drmenzelit
31 Aug 2020

Pull Request for Issue #30426 .

Summary of Changes

Added a new scss file to collect all stuff related with modules
Added a display: grid (with flex fallback) to the class .mod-articlesnews-horizontal

Testing Instructions

Install PR and run npm ci or compile template scss.
Create some articles and a newsflash module with layout horizontal. The width is optimized for 3 articles side by side.

Actual result BEFORE applying this Pull Request

You see all articles as list one below the other

Screenshot_2020-08-31 Home-Newsflash

Expected result AFTER applying this Pull Request

You will see the articles side by side
Screenshot_2020-08-31 Home-Newsflash-horiz
Screenshot_2020-08-31 Home-Newsflash-horiz-2col

Documentation Changes Required

avatar drmenzelit drmenzelit - open - 31 Aug 2020
avatar drmenzelit drmenzelit - change - 31 Aug 2020
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 31 Aug 2020
Category Front End Templates (site) NPM Change
avatar drmenzelit drmenzelit - change - 31 Aug 2020
Labels Added: NPM Resource Changed ?
avatar Magnytu2
Magnytu2 - comment - 31 Aug 2020

Good evening, it's great the choice of vertical or horizontal layout works.
But in the Main-top position, there is a two-column display. In the Top-a position, I have a display of my three articles. But if I choose 4 items, the fourth is on a second line.
I have the impression that there is a limit by the image size.
On the images, we can also see that they are cut which is a shame.
Home-2

avatar drmenzelit
drmenzelit - comment - 31 Aug 2020

As I wrote in my explanation, the width of the grid is optimized for 3 articles side by side. You can overwrite this on your own css. The image has a width of 100% of the column and the height is set to 250px (can be changed too) to avoid having images of different sizes. If the user has always images with the same proportions, it should not be a problem. But I know a lot of users having different sizes for images and then the news will look very weird.

avatar Magnytu2
Magnytu2 - comment - 31 Aug 2020

Sorry, I did not understand this instruction.
It's already very good on three columns, but a pity that it is not proportional.
For the images, we have a huge chance with the new media manager which allows us for example to modify all the images in 16/9.
I don't like templates that cut images because often we don't know how to use them. But that's just my opinion.

avatar chmst chmst - test_item - 1 Sep 2020 - Tested successfully
avatar chmst
chmst - comment - 1 Sep 2020

I have tested this item successfully on cddad60

I have tested this successfully. As it is described, it resolves the issue that the horizontal alignment was not possible. The styling will be a second step and is work in progress.


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

avatar hans2103 hans2103 - test_item - 2 Sep 2020 - Tested successfully
avatar hans2103
hans2103 - comment - 2 Sep 2020

I have tested this item successfully on cddad60


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

avatar infograf768
infograf768 - comment - 2 Sep 2020

Some thoughts.
Should'nt there be a default css for this module?
It would be overridden by any 3rd party template but Cassiopea would use it.

In this case the css could be, or included in a css in the module folder, or an instance created in media/mod_articles_news/css/template.css
This last one through an xml <media destination="mod_articles_news" etc.
or as it is a core module, added to build/ as is done for mod_languages

This would mean that the cassiopea template css would not be crowded with specific modules css when they are not used.

avatar joomla-cms-bot joomla-cms-bot - change - 2 Sep 2020
Category Front End Templates (site) NPM Change Repository NPM Change Modules Front End
avatar drmenzelit
drmenzelit - comment - 2 Sep 2020

I changed the mod_articles_news to use specific css independently from template. The css for mod_articles_news is now under media/mod_articles_news/css (after running npm ci).
I removed the cropping of the images, that can be added on the template if necessary.
I hope my commit is correct, I had some issues with my internet connection the last days :-)

avatar drmenzelit
drmenzelit - comment - 2 Sep 2020

Thank you @infograf768 for the hint with mod_languages :-)

avatar astridx
astridx - comment - 2 Sep 2020

@infograf768 Some thoughts.
Should'nt there be a default css for this module?
It would be overridden by any 3rd party template but Cassiopea would use it.

I think css/scss is the V in MVC and should mainly be the template.

@infograf-768
This would mean that the cassiopea template css would not be crowded with specific modules css when they are not used.

Is this the way @dgrammatiko suggested here:
#30527 (comment) ? Do you mean with "would not be crowded" the place where it is saved or if it is loaded?

avatar infograf768
infograf768 - comment - 3 Sep 2020

Do you mean with "would not be crowded" the place where it is saved or if it is loaded?

Loaded.

avatar infograf768
infograf768 - comment - 3 Sep 2020

I think css/scss is the V in MVC and should mainly be the template.

Agree but "mainly" is the key here. If we have a parameter for a core module, I think it should be taken care of by default in core and obviously allow to be overridden by any template, which is easy as the css is loaded by the module tmpl only when used.

This is the case for mod_languages, plg_editors_codemirror, plg_system_highlight, etc. Some using scss instead of css (why this difference btw? Not sure).

avatar infograf768
infograf768 - comment - 3 Sep 2020

@drmenzelit
Concerning

$wa = $app->getDocument()->getWebAssetManager();
$wa->registerAndUseStyle('mod_modules', 'mod_articles_news/template.css');

I had to place this in /modules/mod_articles_news/tmpl/horizontal.php for the css to load when param is set to horizontal.

Also, I had to change minmax to 240px to get 3 columns instead of 2 here when module is displayed in main-top position and cassiopea is set to static. I understand that maybe voluntary.

I'm a bit confused between use of grid and flex, but that's certainly just me. ;)

avatar drmenzelit
drmenzelit - comment - 4 Sep 2020

@infograf768
The question is if we really need to have a css file for horizontal and one for vertical, or if it is ok to have only one file with all necessary standard definition for the module.

Abot flex and grid, yes, sometimes I'm also confused ;-) The advantage of grid with auto-fit and minmax is that you don't need media queries. But it can be challenging to find the correct size.

avatar infograf768
infograf768 - comment - 4 Sep 2020

No idea if we need one css for vertical, but for sure here your new css have to be loaded in horizontal.php as they don't when loaded through default.php. Maybe one css file only (including vertical and horizontal) needs to be made if needed for vertical, but it should be loaded correctly in each tmpl

avatar drmenzelit
drmenzelit - comment - 4 Sep 2020

@infograf768 yes, you are right! Corrected with the last commit

avatar infograf768 infograf768 - test_item - 4 Sep 2020 - Tested successfully
avatar infograf768
infograf768 - comment - 4 Sep 2020

I have tested this item successfully on daaaae4

Has to be tested again. Needs NPM to test


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

avatar chmst chmst - test_item - 4 Sep 2020 - Tested successfully
avatar chmst
chmst - comment - 4 Sep 2020

I have tested this item successfully on daaaae4


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

avatar Quy Quy - change - 4 Sep 2020
Status Pending Ready to Commit
avatar Quy
Quy - comment - 4 Sep 2020

RTC


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

avatar infograf768 infograf768 - change - 4 Sep 2020
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2020-09-04 17:42:17
Closed_By infograf768
Labels Added: ?
avatar infograf768 infograf768 - close - 4 Sep 2020
avatar infograf768 infograf768 - merge - 4 Sep 2020
avatar infograf768
infograf768 - comment - 4 Sep 2020

Tks

Add a Comment

Login with GitHub to post a comment