Skip to content

Add select2-bootstrap to greenwich theme #18579

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

Merged
merged 5 commits into from
Sep 24, 2020

Conversation

colemanw
Copy link
Member

Overview

Adds bootstrap-3 styling to select2 elements.

Before

image

After

image

@civibot
Copy link

civibot bot commented Sep 24, 2020

(Standard links)

@civibot civibot bot added the master label Sep 24, 2020
@eileenmcnaughton
Copy link
Contributor

nice!

@colemanw
Copy link
Member Author

@totten unfortunately the composer-compile step crashes. Only workaround I've found so far is to manually rename the file bower_components/select2/select2-bootstrap.css and give it an .scss extension.

@@ -5,7 +5,7 @@
"php-method": "\\Civi\\Compile\\Scss::build",
"watch-files": ["scss"],
"scss-files": {"scss/main.scss": "dist/bootstrap3.css"},
"scss-includes": ["scss", "extern/bootstrap3/assets/stylesheets"]
"scss-includes": ["scss", "extern/bootstrap3/assets/stylesheets", "../../bower_components/select2"]
Copy link
Contributor

Choose a reason for hiding this comment

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

@colemanw why is this needed is there an issue with the import if its not there?

@seamuslee001
Copy link
Contributor

@colemanw
Copy link
Member Author

@colemanw does it make more sense to include something like https://github.com/civicrm/civicrm-core/blob/master/ext/greenwich/greenwich.php#L55 ?

Then I think it wouldn't get the #bootstrap-theme namespace prefixing.

@totten
Copy link
Member

totten commented Sep 24, 2020

unfortunately the composer-compile step crashes. Only workaround I've found so far is to manually rename the file bower_components/select2/select2-bootstrap.css and give it an .scss extension.

I didn't realize that select2-bootstrap.css was bundled with select2.

OK, so for completeness, the error that appears to me is:

Fatal error: Uncaught ScssPhp\ScssPhp\Exception\ParserException:
Error : `border-radius: 0 2px 2px 0;` isn't allowed in plain CSS (assign): failed at `}`
../../bower_components/select2/select2-bootstrap.css on line 9, at column 1
in /home/totten/bknix/build/dmaster/web/sites/all/modules/civicrm/vendor/scssphp/scssphp/src/Parser.php:150

Which seems to indicate that (1) ScssPhp does allow importing *.css files but (2) it treats them differently than importing *.scss. This particular error doesn't mean much to me, but it's understandable that different file-extensions would have different import-semantics.

How's this as a fairly straight interpretation of your local work-around:

totten@89bef74

@colemanw
Copy link
Member Author

Yes that's the error message I got too. I tried deleting the "offending" line in the css file and then got the same error for many more lines in the file. To me it looks like a bug in the php scss compiler, but whatever, this workaround works for now.

@colemanw colemanw changed the title WIP: Add select2-bootstrap to greenwich theme Add select2-bootstrap to greenwich theme Sep 24, 2020
@colemanw colemanw added the merge ready PR will be merged after a few days if there are no objections label Sep 24, 2020
1. Using 'php-eval' instead of 'shell' should make it safer to run on Windows.

2. The two steps are very closesly related. Putting them into the same 'task'
   means that it's a bit easier to understand the 'watch' behavior.
@totten
Copy link
Member

totten commented Sep 24, 2020

OK, cool.

@colemanw I have a slightly better form of the same patch now - it's a bit more intuitive to manage the 'watch' list, and I expect it to run better on Windows. It needs an update version of compile-plugin v0.8. Changes pushed onto the PR.

@colemanw
Copy link
Member Author

@totten I tested it and it works well.

@colemanw colemanw merged commit 01e1cde into civicrm:master Sep 24, 2020
@colemanw colemanw deleted the select2-bootstrap branch September 24, 2020 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master merge ready PR will be merged after a few days if there are no objections
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants