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

Material UI implementation #21

Merged
merged 29 commits into from Oct 17, 2019
Merged

Material UI implementation #21

merged 29 commits into from Oct 17, 2019

Conversation

gverni
Copy link
Contributor

@gverni gverni commented May 15, 2019

Material UI theme implementation. Feel free to contribute.

TODO / check

  • Confirm button
  • Cancel button
  • Icons for different pop-up types: No changes
  • Textbox: no changes (outlined style with round corners and no label). We may just look into make the validation error change color of the input (but that may require changes to Swal2 js)
  • Select: implemented with CSS background image. To be visually tested on different browsers
  • Radio
  • Checkbox
  • File: No changes
  • Range
  • Toast
  • Close icon
  • Validation message: No changes
  • Queues: No changes
  • Action buttons focus

Close #14

@gverni
Copy link
Contributor Author

gverni commented May 23, 2019

I've been wrestling with the close button for the past couple of days. Unfortunately when the font-size is increased, the × character moves slightly down and that's what you get when you hover on the close button:

image
(the X is not centred)

I don't really like the effect, so I may drop completely the hover style (need to check if this is mandatory on material) or replace the × with an SVG. @limonte would you be in favour of this latter (i.e. use SVG for close button)?

@gverni
Copy link
Contributor Author

gverni commented May 23, 2019

Thanks @limonte! I pushed it in 2e0a5de. Any help is appreciated.

P.s. Sorry I pushed them from the wrong account and I had to force push them again

@limonte
Copy link
Member

limonte commented May 23, 2019

I've been wrestling with the close button for the past couple of days. Unfortunately when the font-size is increased, the × character moves slightly down and that's what you get when you hover on the close button:

The same issue is happening for the default style, but it's not obvious because there's no background color on the close button.

Decreasing the line-height from 1.2 to 1.15 seems to fix the vertical alignment for me (Chrome and Firefox on Linux). Does it help for you as well?

@gverni
Copy link
Contributor Author

gverni commented May 24, 2019

Decreasing the line-height from 1.2 to 1.15 seems to fix the vertical alignment for me (Chrome and Firefox on Linux). Does it help for you as well?

On chrome on macOS line-height seems to have no effect at all on buttons. On the other hand It applies correctly to div. Maybe the user agent stylesheet is different between browsers / OSs.

Here you can find a codepen with the following elements:

<div class="swal-close-manual">&times;</div>
<button class="swal-close-manual">&times;</button>

For both the swal-close-manual is setting line-height: 0.6; but on my browser this is applied only to div:

image

Could you have a try on linux? Or did I misunderstand your suggestion?

Using CSS background property, the circle on hover is slightly
misaligned. To workaround that, an SVG circle is used as background
image. The circle is then moved slightly down to be centered around
the X.
@gverni
Copy link
Contributor Author

gverni commented May 28, 2019

On chrome on macOS line-height seems to have no effect at all on buttons.

@limonte I tried a different approach in 579040e. I'm here using an SVG circle as background image, with a position offset down (background-position top set at 115%). What do you think of it?

@limonte
Copy link
Member

limonte commented May 29, 2019

@limonte I tried a different approach in 579040e. I'm here using an SVG circle as background image, with a position offset down (background-position top set at 115%). What do you think of it?

Good day @gverni!

  1. the solution looks complex for its task
  2. the vertical alignment is still broken in Chrome on Linux:

image

@gverni
Copy link
Contributor Author

gverni commented May 29, 2019

  1. the vertical alignment is still broken in Chrome on Linux:

Thanks @limonte for checking. I'll try some different approach and let you know.

@gverni
Copy link
Contributor Author

gverni commented May 29, 2019

  1. the solution looks complex for its task

@limonte just to check: in general, are you ok with using SVG inside CSS? I'm asking because this is what I used to style the select element in c655575. Let me know if you are against it in principle because I may have to use them to style other elements for material ui.

@limonte
Copy link
Member

limonte commented May 29, 2019

@limonte just to check: in general, are you ok with using SVG inside CSS?

Yup, I'm fine with SVG inside CSS. What I don't like is so-called magic numbers, which are hard to understand and maintain:

image

@gverni
Copy link
Contributor Author

gverni commented May 29, 2019

What I don't like is so-called magic numbers, which are hard to understand and maintain

Fair enough @limonte 😉 I'll definitely avoid that. I'm setting up a linux machine so I'll avoid bothering you for testing (unfortunately Sauce Labs doesn't support latest version for Chrome/Firefox on Linux)

@gverni gverni mentioned this pull request Jun 4, 2019
There are a lot of "magic number" so we need to  review this
@gverni
Copy link
Contributor Author

gverni commented Jun 11, 2019

@limonte I added styling for radio element. Note that there are some magic numbers (I can review them later) and also i'm not using any Swal2 SCSS variables. What do you think of this approach?

In general for forms elements, I found difficult to style them without using background images (as in c655575) or pseudo element (as in 9e342dd)

@limonte
Copy link
Member

limonte commented Jun 11, 2019

@gverni I totally understand difficulties you're facing, Material theme is the most complex theme to implement.

I'm alright with the PR so far, all those small details you mentioned could be improved later if needed.

Previous code was adding the radio control also for checkboxes
@gverni
Copy link
Contributor Author

gverni commented Jun 20, 2019

@Yairsaenz7 note that I just pushed a fix for a bug I introduced for the radio button and that was affecting the checkboxes as well. Please pull the latest commit on this PR.

Added using engine pseudo-elements
@gverni
Copy link
Contributor Author

gverni commented Sep 8, 2019

Couple of considerations on the last commits (be76d8e & c691cc0):

  • Based on material-UI spec, the focus colour should be slightly darker than the hover. The active seems to be similar color as the focus
  • Added SCSS variable $swal2-button-focus-background. We may want to add this back to the main SA2 SCSS
  • Side effect of the commit is that the action button(s) looks darker on modal opening (since they are autofocused)

The serif font-family is not vertically aligned on macOS.
Arial font-family is better vertically aligned
@gverni
Copy link
Contributor Author

gverni commented Sep 29, 2019

In order to keep alignment of the hover background, the font-family for close button have been changed to Arial. Below how the serif is rendered vs the arial on macOS:

  • Arial
    image

  • Serif
    image

@gverni gverni marked this pull request as ready for review September 29, 2019 11:34
@gverni
Copy link
Contributor Author

gverni commented Sep 29, 2019

After 4 and half months, this PR is (finally) ready for review...

Commit 7f14f5c (PR) was created without taking
into consideration commit 76aacc2
Need to be same color as `focus`
The serif font-family is not vertically aligned on macOS.
Arial font-family is better vertically aligned
@gverni
Copy link
Contributor Author

gverni commented Sep 29, 2019

OK.. now it's ready (had to merge some critical changes from master)

@maicol07
Copy link

Is this completed? @gverni and @limonte

@gverni
Copy link
Contributor Author

gverni commented Oct 17, 2019

@maicol07 development is completed, but we need some more testing and feed-back. Feel free to try it out and give us your feed-back!

@maicol07
Copy link

How can I test?

@limonte limonte merged commit 77e5513 into master Oct 17, 2019
@limonte limonte deleted the feat/material-ui branch October 17, 2019 16:54
limonte pushed a commit that referenced this pull request Oct 17, 2019
# [2.2.0](v2.1.0...v2.2.0) (2019-10-17)

### Features

* Material UI theme ([#21](#21)) ([77e5513](77e5513))
@limonte
Copy link
Member

limonte commented Oct 17, 2019

🎉 This PR is included in version 2.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

limonte pushed a commit that referenced this pull request Nov 6, 2022
* Add material-ui folder and files

* Add confirm button style

* Add cancel button style

* Replace "none" with 0 opacity color

To override buttons background color

* Use new flex justify variable

The variable have been introduced in sweetalert version 8.11.1

* Set color for disabled buttons

* Upgrade sweetalert2 dependency version

* Add hover for close button

* Add style for select

* Implement hover background for close

Using CSS background property, the circle on hover is slightly
misaligned. To workaround that, an SVG circle is used as background
image. The circle is then moved slightly down to be centered around
the X.

* Add error icon for text box

* Add radio input styling

There are a lot of "magic number" so we need to  review this

* Adjust poisition of swal2-label stylying for radio

Previous code was adding the radio control also for checkboxes

* Add style for range / slider

Added using engine pseudo-elements

* Apply material UI style to checkbox

Apply material UI style to checkbox

* Fix merge issue

* Fix label padding merge issue

Commit 7f14f5c (PR) was created without taking
into consideration commit 76aacc2

* Implement focus for action buttons

* Darken active action button background

Need to be same color as `focus`

* Set font-family for the close button

The serif font-family is not vertically aligned on macOS.
Arial font-family is better vertically aligned

* Merge conflicts with master

* Fix label padding merge issue

Commit 7f14f5c (PR) was created without taking
into consideration commit 76aacc2

* Implement focus for action buttons

* Darken active action button background

Need to be same color as `focus`

* Set font-family for the close button

The serif font-family is not vertically aligned on macOS.
Arial font-family is better vertically aligned

* fix lint errors

* remove .sass-lint.yml
limonte pushed a commit that referenced this pull request Nov 6, 2022
# [2.2.0](v2.1.0...v2.2.0) (2019-10-17)

### Features

* Material UI theme ([#21](#21)) ([9a7b7cc](9a7b7cc))
limonte pushed a commit that referenced this pull request Nov 6, 2022
* Add material-ui folder and files

* Add confirm button style

* Add cancel button style

* Replace "none" with 0 opacity color

To override buttons background color

* Use new flex justify variable

The variable have been introduced in sweetalert version 8.11.1

* Set color for disabled buttons

* Upgrade sweetalert2 dependency version

* Add hover for close button

* Add style for select

* Implement hover background for close

Using CSS background property, the circle on hover is slightly
misaligned. To workaround that, an SVG circle is used as background
image. The circle is then moved slightly down to be centered around
the X.

* Add error icon for text box

* Add radio input styling

There are a lot of "magic number" so we need to  review this

* Adjust poisition of swal2-label stylying for radio

Previous code was adding the radio control also for checkboxes

* Add style for range / slider

Added using engine pseudo-elements

* Apply material UI style to checkbox

Apply material UI style to checkbox

* Fix merge issue

* Fix label padding merge issue

Commit 7f14f5c (PR) was created without taking
into consideration commit 76aacc2

* Implement focus for action buttons

* Darken active action button background

Need to be same color as `focus`

* Set font-family for the close button

The serif font-family is not vertically aligned on macOS.
Arial font-family is better vertically aligned

* Merge conflicts with master

* Fix label padding merge issue

Commit 7f14f5c (PR) was created without taking
into consideration commit 76aacc2

* Implement focus for action buttons

* Darken active action button background

Need to be same color as `focus`

* Set font-family for the close button

The serif font-family is not vertically aligned on macOS.
Arial font-family is better vertically aligned

* fix lint errors

* remove .sass-lint.yml
limonte pushed a commit that referenced this pull request Nov 6, 2022
# [2.2.0](v2.1.0...v2.2.0) (2019-10-17)

### Features

* Material UI theme ([#21](#21)) ([a782cac](a782cac))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Material UI theme
4 participants