Fix buggy placement of icons on buttons

[agh1]

Overview

Buttons with icons can look funny in a number of circumstances, and CiviCRM wasn't always consistent with itself on how icons were placed. Specifically, icons had absolute positioning 4 pixels from the top and left of the button. For many sites, this would be well above or below the text and could distort the button size.

This standardizes icons as inline blocks with standard padding and a single space between them and any text that follows. In the process, I simplified a lot of how the box model on buttons is managed.

Finally, the WordPress admin CSS gives items with the class button a minimum height of 30 pixels, so this does a reset of that.

Comparison

Context	Before	After
Stock Drupal
Drupal w/ button height at 2em
Drupal w/ Shoreditch
WordPress

(In the View Contribution screenshots, the first two are <a class="button"> while the third is an actual <input type="submit">.)

Technical Details

There was a special class for buttons with icons, and this makes that unnecessary. There were also some unused or barely-used button and icon related classes that I removed, standardizing the markup if necessary.

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[agh1]

@colemanw I wanted to be sure you saw this because it undoes some of what you added back in #7199. Please jump in if you think it's better to position the icons absolutely rather than inline with the text.

[MegaphoneJon]

@agh1 and whoever reviews this - please make sure to check in both Shoreditch and non-Shoreditch. #17295 caused "More" menus to disappear out of Shoreditch, and while it seems to be more about Shoreditch's assumptions than Andrew's PR, we should make an effort to include it in our testing.

[agh1]

@MegaphoneJon thanks for suggesting--I just added screenshots with Shoreditch, which appears to have nixed the icons all along.

I do think we're in a bit of an unstable situation with regard to the UI. On the one hand, you can reasonably complain that a core change breaks Shoreditch, which has been promoted as "the future" in some circles for a while. Meanwhile, others can reasonably complain that something like #17775 causes core to depend upon Shoreditch, or at least Bootstrap.

Finally, a key effort has been that the work to enable Shoreditch / CiviHR / "new" CiviCase should be modular so that others can write their own themes, and others have been doing just that. It's sort of reasonable to expect that a PR works nicely in four or five CMS contexts plus a second theme for one of them, but it becomes unreasonable to expect testing of a growing number of themes.

One of the driving forces behind dev/user-interface#24 is dissatisfaction with Shoreditch, and I do think it will probably remain a niche thing rather than an emerging standard.

[agh1]

Jenkins retest this please

[eileenmcnaughton]

This is too form-layery to need a test

[eileenmcnaughton]

@agh1 conflict....

[agh1]

@eileenmcnaughton thanks, just rebased

[eileenmcnaughton]

@vingle @christianwach @artfulrobot @colemanw is anyone able to review this?

Regarding shoreditch - affecting shoreditch is non-blocking but we should at least make @jamienovick aware when we think a PR may affect shoreditch so he can adjust shoreditch.

[agh1]

I just reworked the screenshots into a table so it's easier to see changes side-by-side in each context. Also, this is a case where a reviewer might do best by looking at the commits in sequence. Some are more significant than others, and 90% of the file changes are just one commit that cleans up extra non-breaking spaces.

[vingle]

Can confirm that the CSS isn't problematic & the fix works for Joomla:

Before (with 3em height for emphasis)

After



[seamuslee001]

Merging based on @vingle 's review here

[demeritcowboy]

I'm not sure if it's this PR but I'm noticing that one of the icon/button PRs lately changes the clickable surface area of the buttons. So for example if you click on a button in the area where the icon is then nothing happens, but if you click on the word then it clicks. Whereas hovering still covers the whole button surface.

[agh1]

@demeritcowboy ugh you are absolutely right, and this PR caused it. The issue is that the button isn't what you see--it's only the part with the text. This all stems from using <input type="submit"> rather than <button type="submit">: the latter allows content like icons while the former does not. That would be the most natural solution; otherwise, we might need to revert.

[mattwire]

@agh1 @demeritcowboy We have some good forward-progress on buttons so would be a shame to revert if we can continue to improve. I've noticed that the clickable area is smaller and also once submitted they go a strange hashed colour briefly. I know that @vingle is keen to review button PRs (and see them become more consistent).

[agh1]

@demeritcowboy @mattwire I'm making some progress on getting all of these switched out for real <button> elements. They'll need to be reworked to display right (our CSS doesn't really anticipate actual <button> elements), but the end result will be much more consistent with the other pseudo-buttons since they can have actual content.

[agh1]

@demeritcowboy @mattwire @vingle I have a WIP that replaces all of the <input type="submit"> elements with <button type="submit">. It's not ready to review because I need to fix the appearance. See #18087 if you want to look at what's going on.

[demeritcowboy]

Thanks @agh1

[vingle]

This is very promising - be brilliant to see the end of <input type="submit">.

[eileenmcnaughton]

per recommendation from @mattwire this has been reverted along with the previously reverted one since it seems required to get stable. The plan is to send out an email before the end of the week pointing people to both & aim to merge next week into what will be 5.31