Skip to content
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] Remove "cardGrey" chrome #30734

Merged
merged 1 commit into from Sep 24, 2020
Merged

Conversation

Bakual
Copy link
Contributor

@Bakual Bakual commented Sep 22, 2020

As written in #30729, the "cardGrey" chrome is basically the same chrome as the "card" (once #30729 is merged) chrome. It just has an added "card-grey" module class.

However the same can be achieved by using the "module class" parameter in the module options and add "card-grey" there.

Summary of Changes

  • Removes the chrome layout /templates/cassiopeia/html/layouts/chromes/cardGrey.php
  • Changes the styles of the positions which were using it to "card". That is "top-a", "main-top", "main-bottom" and "bottom-a".

Testing Instructions

  • Make sure to have the other PR applied as well, otherwise the chrome will not be found and you get some ugly display. Since it's RTC already, chances are high it's merged soon.
  • Add the class "card-grey" to the module class parameter in your module

Actual result BEFORE applying this Pull Request

No change as the class is there anyway, now twice.

Expected result AFTER applying this Pull Request

Position gets the darker background only if that class is added to the module, otherwise the regular white background is used.

Documentation Changes Required

None

…g the module class "card-grey" with the "card" chrome
@richard67
Copy link
Member

I have tested this item ✅ successfully on a498053


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

@richard67
Copy link
Member

Hint for other testers: With patchtester you can't apply both PRs #30729 and this one here. But you can apply #30729 and then do the changes from this PR here by editing the files.

@Bakual
Copy link
Contributor Author

Bakual commented Sep 23, 2020

The other PR is now merged, so tomorrow in the nightly.

@richard67
Copy link
Member

richard67 commented Sep 23, 2020

@Bakual Please update the branch of this PR to latest 4.0-dev by merging the 4.0-dev branch of the CMS repo into it.

Update: That's needed for being able to test this PR here with patchtester.

@infograf768
Copy link
Member

I have tested this item ✅ successfully on a498053


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

@infograf768
Copy link
Member

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Sep 23, 2020
@richard67
Copy link
Member

@Bakual Should there be some documentation about the effect adding the class "card-grey" to the module class parameter? Or is that such a standard bootstrap thing that everybody should know who deals with bootstrap based templates?

@Bakual
Copy link
Contributor Author

Bakual commented Sep 23, 2020

That's a good question and I don't know the answer. I just assumed it was a standard Bootstrap class, but when I check the Bootstrap documentation (https://getbootstrap.com/docs/4.0/components/card/) there are no specific colour styling classes for cards.
The BS class which could be used is "bg-light" (from https://getbootstrap.com/docs/4.0/utilities/colors/) which is a tiny bit lighter than "card-grey".

The class "card-grey" seems to be a Cassiopeia class defined in https://github.com/joomla/joomla-cms/blob/4.0-dev/templates/cassiopeia/scss/vendor/bootstrap/_card.scss (together with "card-inverse" which seems to be unused).
Maybe that file could be removed - but that's a question for people who know how that stuff works. I just see that it was part of the initial template PR by @C-Lodder

@richard67
Copy link
Member

Well, maybe just add something to the "Documentation Changes Required" section of this PR, telling that it needs to document somewhere the Cassiopeia module classes which can be used, and we set the documentation required label. That will not stop this PR from being merged, and we have a reminder somewhere.

@Bakual
Copy link
Contributor Author

Bakual commented Sep 23, 2020

Personally I would just remove the cassiopeia specific styling for card-grey and people can use any of the default BS classes instead (or add own ones).

@C-Lodder
Copy link
Member

@Bakual
Copy link
Contributor Author

Bakual commented Sep 23, 2020

I'll leave this PR as is. I can try to remove the SCSS rules in another PR, as soon as I have figured out how to recreate all the compiled CSS files 😄

@Bakual
Copy link
Contributor Author

Bakual commented Sep 24, 2020

See #30754 for the removal of the scss file.

@wilsonge wilsonge merged commit fdb111c into joomla:4.0-dev Sep 24, 2020
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Sep 24, 2020
@wilsonge
Copy link
Contributor

Thanks!

@wilsonge wilsonge added this to the Joomla 4.0 milestone Sep 24, 2020
@Bakual Bakual deleted the 4_RemoveCardGreyChrome branch September 24, 2020 20:54
dgrammatiko added a commit to dgrammatiko/joomla-cms that referenced this pull request Sep 26, 2020
* Joomla/4.0-dev: (844 commits)
  [4.0] Template layout select (joomla#30772)
  [4.0][CLI] com_finder use console command (joomla#30768)
  [4.0] Modifying com_actionlogs string (joomla#30758)
  [4.0] Fancy selectbox fix (joomla#30739)
  [4.0] Add missing Table Caption (joomla#30763)
  [4.0] Wrap all buttons in btn-group to improve styling (joomla#30761)
  [4.0] Cassiopeia missing string (joomla#30765)
  Improve batch text (joomla#28447)
  Fix icons not displaying (joomla#30749)
  Remove the chrome "cardGrey". The same effect can be achieved by using the module class "card-grey" with the "card" chrome (joomla#30734)
  Remove obsolete html code (joomla#30737)
  [4.0] Check out improvements related to nullable columns (joomla#30747)
  Removing card.scss overrides as we don't use those classes
  Use renderField() method to render fields (joomla#30738)
  [4.0] spelling (joomla#30742)
  [4.0] Remove use of ReflectionParameter::getClass() (joomla#30581)
  [4.0] Fix saving images (joomla#30730)
  [4.0] Cleanup cassiopeia chromes and rename "default" to "card" (joomla#30729)
  Fix hiddenLabel and add hiddenLegend form attribute (joomla#29710)
  [4.0] Fix Debug plugin to display query parameters (joomla#30717)
  ...
dgrammatiko added a commit to dgrammatiko/joomla-cms that referenced this pull request Sep 29, 2020
…ger_events_consistency

* '4.0-dev' of github.com:joomla/joomla-cms: (84 commits)
  [4.0] Error in legacy plugins when method contains $event argument (joomla#30575)
  [4.0] Cassiopea table css (joomla#30740)
  [4.0] Blog view links (joomla#30788)
  Change grid minmax definition for newsflash module (joomla#30781)
  [4.0] Pluginless lazyloading for the core (joomla#30748)
  Update package-lock.json (joomla#30713)
  [4.0] mod_article_news readmore (joomla#30780)
  Improve code, remove separator (joomla#30785)
  [4.0] Template layout select (joomla#30772)
  [4.0][CLI] com_finder use console command (joomla#30768)
  [4.0] Modifying com_actionlogs string (joomla#30758)
  [4.0] Fancy selectbox fix (joomla#30739)
  [4.0] Add missing Table Caption (joomla#30763)
  [4.0] Wrap all buttons in btn-group to improve styling (joomla#30761)
  [4.0] Cassiopeia missing string (joomla#30765)
  Improve batch text (joomla#28447)
  Fix icons not displaying (joomla#30749)
  Remove the chrome "cardGrey". The same effect can be achieved by using the module class "card-grey" with the "card" chrome (joomla#30734)
  Remove obsolete html code (joomla#30737)
  [4.0] Check out improvements related to nullable columns (joomla#30747)
  ...

� Conflicts:
�	administrator/components/com_media/resources/scripts/app/Api.js
�	administrator/components/com_media/resources/scripts/components/toolbar/toolbar.vue
�	package-lock.json
sakiss pushed a commit to sakiss/joomla-cms that referenced this pull request Oct 16, 2020
…g the module class "card-grey" with the "card" chrome (joomla#30734)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants