Skip to content

[4.0] minicolors #27829

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.

Sign up for GitHub

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

Merged
wilsonge merged 1 commit into from
Apr 11, 2020
Merged

[4.0] minicolors #27829

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion administrator/templates/atum/scss/template.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,11 @@
// Fonts
@import "../../../../media/vendor/roboto-fontface/scss/roboto/sass/roboto-fontface";


@import "blocks/global"; // Leave this first

// jQuery Minicolors
@import "../../../../build/media_source/system/scss/jquery-minicolors";
Copy link
Copy Markdown
Contributor

@dgrammatiko dgrammatiko Feb 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wrong! There is already a css file so either override it or extend it. The template shouldn't be aware of every 3rd pd extension that might be used in some pages, Joomla has specific layouts for these (input colour advanced for this one iirc)...

Copy link
Copy Markdown
Contributor Author

@brianteeman brianteeman Feb 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So why is the file there and why is it used in Cassiopeia?

Getting really sick of you telling everyone they are wrong without contributing. You're starting to be a troll.

Copy link
Copy Markdown
Contributor

@dgrammatiko dgrammatiko Feb 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getting really sick of you telling everyone they are wrong without contributing

This is funny I did #26077 so people can follow up and fix the rest and now you're undoing it instead of fixing whatever bug. Do you expect me to fix this ugly spaghetti of css alone?

So why is the file there and why is it used in Cassiopeia?

If it's embedded in the template's css then it's wrong. Joomla is modularised by default making it a freaking monolith is wrong, but hey it's working.

Copy link
Copy Markdown
Contributor Author

@brianteeman brianteeman Feb 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you missed the fact that this PR was to fix an issue raised by someone with far more experience than me at scss.

I guess you missed the fact that I specifically asked where to put the code and the same person with far more experience than me at scss told me where to put it. Which is exactly what I did.

Copy link
Copy Markdown
Contributor

@dgrammatiko dgrammatiko Feb 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoever told you to put the css there IS WRONG. Joomla by default is modular, with this kind of code snippets you're making it monolith. Prove me wrong!

Copy link
Copy Markdown
Contributor

@dgrammatiko dgrammatiko Feb 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be quite clear here: I'm arguing with this approach because you're effectively undoing #26077


// Vendor overrides
@import "vendor/bootstrap/badge";
@import "vendor/bootstrap/buttons";
Expand Down
6 changes: 5 additions & 1 deletion build/media_source/system/scss/_jquery-minicolors.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
// Overrides for jQuery Minicolors

.minicolors-theme-bootstrap .minicolors-swatch {
.minicolors-theme-bootstrap .minicolors-swatch>.minicolors-sprite {
top: 50%;
left: 8px;
border-radius: 0;
transform: translateY(-50%);
}

span.minicolors-swatch-color {
cursor: pointer;
}