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

[5.0] Fix broken SCSS compilation #41566

Closed
wants to merge 1 commit into from

Conversation

richard67
Copy link
Member

@richard67 richard67 commented Sep 3, 2023

Pull Request for https://github.com/joomla/joomla-cms/pull/41409/files#r1314271898 .

Summary of Changes

Fix SCSS compilation which was broken with #41409 .

Maybe it needs more to be fixed, so let's wait for drone.

Testing Instructions

Run npm ci or check the NPM step on drone.

Actual result BEFORE applying this Pull Request

Error: Undefined variable.
   ╷
12 │ $form-select-background-dark:      $form-select-indicator-dark no-repeat right center / $form-select-bg-size; // Used so we can have multiple background elements (e.g. arrow and feedback icon)
   │                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^
   ╵
  build/media_source/templates/administrator/atum/scss/_variables-dark.scss 12:36  @import

Expected result AFTER applying this Pull Request

Success.

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-5.0-dev labels Sep 3, 2023
@richard67
Copy link
Member Author

@dgrammatiko Could you check if it needs more to be fixed from PR #41409 ?

@richard67
Copy link
Member Author

NPM still fails. It seems the $form-select-indicator-dark and possibly others are not defined.

@brianteeman
Copy link
Contributor

This si wrong. You said the variable is not defined and then still use the variable

@richard67
Copy link
Member Author

This si wrong. You said the variable is not defined and then still use the variable

I know. It was a test if there is a syntax error. Closing as it seems to be wrong also regarding the purpose of that CSS, and hoping @dgrammatiko will be available to check and provide a fix.

@richard67 richard67 closed this Sep 3, 2023
@richard67 richard67 deleted the 5.0-dev-fix-broken-scss branch September 3, 2023 15:04
@Fedik
Copy link
Member

Fedik commented Sep 3, 2023

Need to swap lines:

$form-select-indicator-dark:       url("../images/select-bg-dark.svg");
$form-select-indicator-rtl-dark:   url("../images/select-bg-rtl-dark.svg");
$form-select-bg-dark:              var(--template-bg-dark);
$form-select-background-dark:      $form-select-indicator-dark no-repeat right center / $form-select-bg-size; // Used so we can have multiple background elements (e.g. arrow and feedback icon)
$form-select-background-rtl-dark:  $form-select-indicator-rtl-dark no-repeat left center / $form-select-bg-size; // Used so we can have multiple background elements (e.g. arrow and feedback icon)

However there some more issues

@richard67
Copy link
Member Author

@Fedik I only need npm to complete without error in the 5.0-dev branch for the other PR's which I currently am helping with, the task scheduler stuff from Nicola.

@dgrammatiko
Copy link
Contributor

@Fedik
Copy link
Member

Fedik commented Sep 3, 2023

I tried another one #41567

Try using a css var like I do in the muta

That something like rewrite, not fixing 😄

@dgrammatiko
Copy link
Contributor

@Fedik: well, what do I know 😀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NPM Resource Changed This Pull Request can't be tested by Patchtester PR-5.0-dev
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants