Skip to content

[4.0] [Cassiopeia] Remove header SVG curve#27566

Merged
infograf768 merged 8 commits intojoomla:4.0-devfrom
ciar4n:front-remove-curve
Jan 21, 2020
Merged

[4.0] [Cassiopeia] Remove header SVG curve#27566
infograf768 merged 8 commits intojoomla:4.0-devfrom
ciar4n:front-remove-curve

Conversation

@ciar4n
Copy link
Copy Markdown
Contributor

@ciar4n ciar4n commented Jan 19, 2020 •
edited
Loading

Pull Request for Issue #27565 .

Summary of Changes

Removes the SVG curve from the frontend template header

Testing Instructions

Apply patch and run node build.js --compile-css to update the changed SCSS.

Add banner module to banner position

Before PR

Banner cropped by SVG

After PR

Banner is visible

Documentation Changes Required

No

@ciar4n ciar4n mentioned this pull request Jan 19, 2020
Closed
@infograf768
Copy link
Copy Markdown
Member

infograf768 commented Jan 19, 2020

Great.
Remains one aspect: the Module Class. What do we do with that?
Screen Shot 2020-01-19 at 12 34 18

@ciar4n
Copy link
Copy Markdown
Contributor Author

ciar4n commented Jan 19, 2020

Not sure I understand what you mean. Is it missing from the output?

@infograf768
Copy link
Copy Markdown
Member

infograf768 commented Jan 19, 2020

Is it missing from the output?

As far as I see , yes.

@Quy Quy mentioned this pull request Jan 19, 2020
Closed
@infograf768
Copy link
Copy Markdown
Member

infograf768 commented Jan 19, 2020

I think it comes from
<jdoc:include type="modules" name="banner" style="xhtml" />

Cardgrey and default chromes contain <?php echo htmlspecialchars($params->get('moduleclass_sfx')

not xhtml

@ciar4n
Copy link
Copy Markdown
Contributor Author

ciar4n commented Jan 19, 2020 •
edited
Loading

I've changed the style to html5 which appears to be the only chrome with no styling and adds the class. It appears default applies card styling to the module which is incorrect imo and possibly needs changing (separate issue/pr).

@infograf768
Copy link
Copy Markdown
Member

infograf768 commented Jan 19, 2020

On ipad, will test later.
In j3, chromes are defined in system.
Except for the specific chromes which are defined in the template itself, do you have an idea where are defined html5 and xhtml in j4 ?

@ciar4n
Copy link
Copy Markdown
Contributor Author

ciar4n commented Jan 19, 2020

System chromes are in ../layouts/chromes/

@infograf768
Copy link
Copy Markdown
Member

infograf768 commented Jan 19, 2020

I have tested this item ✅ successfully on bdcce04

Looks fine here.

we get <div class="moduletable mybannerclass"> which is fine.

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

@infograf768
Copy link
Copy Markdown
Member

infograf768 commented Jan 20, 2020

@ciar4n
Looks like we have also another xhtml style in error.php

<?php if ($this->countModules('banner')) : ?>
		<div class="grid-child container-banner">
			<jdoc:include type="modules" name="banner" style="xhtml" />
		</div>

@infograf768 infograf768 mentioned this pull request Jan 20, 2020
Merged
@ChristineWk
Copy link
Copy Markdown

ChristineWk commented Jan 20, 2020 •
edited
Loading

banner
I have tested this item ✅ successfully on aee0cc4

Tested hours before, but after applying Patch, I got error 404 and: Call to undefined method Joomla\CMS\WebAsset\WebAssetManager::usePreset()
Then I found: brianteeman@7fa8d42
My site was from oct2019, therefore checked above files. Finally installed now completely new nightly build from today.

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

@jwaisner
Copy link
Copy Markdown
Member

jwaisner commented Jan 21, 2020

I have tested this item ✅ successfully on aee0cc4

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

@infograf768
Copy link
Copy Markdown
Member

infograf768 commented Jan 21, 2020

RTC

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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jan 21, 2020
@infograf768 infograf768 merged commit f0432e8 into joomla:4.0-dev Jan 21, 2020
@infograf768 infograf768 added this to the Joomla 4.0 milestone Jan 21, 2020
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jan 21, 2020
@infograf768
Copy link
Copy Markdown
Member

infograf768 commented Jan 21, 2020

tks

@ciar4n ciar4n deleted the front-remove-curve branch January 26, 2020 16:30
@Scrabble96
Copy link
Copy Markdown
Contributor

Scrabble96 commented Jan 31, 2020

Sorry, I know the original issue was posted by me, but I'm unable to test this; I'm waiting for a new computer :-( which should arrive sometime next week.

brianteeman pushed a commit to brianteeman/joomla-cms that referenced this pull request Feb 4, 2020
@ciar4n @infograf768 @brianteeman
[4.0] [Cassiopeia] Remove header SVG curve (joomla#27566)
dccb96f 
* [4.0] [Cassiopeia] Remove header SVG curve

* change banner pos style to html5

* color white

* give p tag back it's margin

Co-authored-by: infograf768 <infografjms@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

Joomla 4.0

Development

Successfully merging this pull request may close these issues.

6 participants

@ciar4n @infograf768 @ChristineWk @jwaisner @Scrabble96 @joomla-cms-bot