civicrm / org.civicrm.shoreditch

Other
45 stars 59 forks source link

CPS-322: Support new Button markup #473

Closed deb1990 closed 4 years ago

deb1990 commented 4 years ago

Overview

Recently in the Civicrm Core(https://github.com/civicrm/civicrm-core/pull/18410), all the input type button markups has been changed to button elements. This caused a lot of styles to break when using Shoreditch theme. So this PR fixes those UI related style issues.

Before (Only showing some examples)

2020-10-19 at 3 08 PM

2020-10-19 at 3 09 PM

2020-10-19 at 3 10 PM

After (Only showing some examples)

2020-10-19 at 3 12 PM

2020-10-19 at 3 12 PM

2020-10-19 at 3 11 PM

Technical Details

  1. Changed css selector from input to button in

    scss/civicrm/administer/contribute/_managepremiums.scss
    scss/civicrm/common/_buttons.scss
    scss/civicrm/contact/pages/_contributions.scss
    scss/civicrm/financial/_batchtransaction.scss
    scss/civicrm/search/pages/_full-text-search.scss
  2. Made adjustments to the following files to support new markup

    
    // There are no extra parent divs for buttons, so moved the css one level up
    scss/bootstrap/overrides/crm/_button.scss 

// no extra css required for .crm-i, as items inside buttons are auto aligned scss/civicrm/administer/display/_display.scss

// styles for .crm-i removed, as items inside buttons are auto aligned, and crm-i-button does not exist anymore scss/civicrm/search/pages/_advanced-search.scss



3. Added styles specific to Case Types page in `scss/civicrm/administer/civi-case/_case-types.scss`. And this code was moved from `scss/civicrm/administer/common/_common.scss `. 
This style was moved to the `common.scss` in https://github.com/civicrm/org.civicrm.shoreditch/pull/49. But now it is causing regressions in other pages. And other pages are working fine without these changes, so made it only for case type page.

## Comments
Backstop JS and manual test suites were run to ensure no regressions are present.
eileenmcnaughton commented 4 years ago

Just noting I tried this & it fixed the issue I see on 5.31 at

http://dmaster.local/civicrm/admin/uf/group/update?action=update&id=1&context=group

Screen Shot 2020-10-08 at 8 58 56 AM

vs

Screen Shot 2020-10-08 at 9 00 37 AM
deb1990 commented 4 years ago

@eileenmcnaughton Thanks for checking. PR should be ready within the next week.

deb1990 commented 4 years ago

@deb1990 also, why did the package-lock.json file change?

I never knew how this works, but now I have a answer https://stackoverflow.com/a/50868741. The package-lock.json is supposed to be updated every time you run npm install. Otherwise we need to run npm ci to download exact versions. So in development its fine to use npm install and in production npm ci should be used.

reneolivo commented 4 years ago

@deb1990 can you please use nvm use and then npm install ? the package-lock.json is not supposed to change when installing local packages. The only reason for changing is when installing using a different Node version than the one enforced by NVM/NPM.

eileenmcnaughton commented 4 years ago

5.31 drops tomorrow so you probably want to merge this & tag a 5.31 compatible release

deb1990 commented 4 years ago

@eileenmcnaughton Thanks for the reminder. It will be merged today.

deb1990 commented 4 years ago

@eileenmcnaughton This is the release to support CiviCRM 5.31 buttons. https://github.com/civicrm/org.civicrm.shoreditch/releases/tag/1.0.0-beta.4

eileenmcnaughton commented 4 years ago

yay