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.

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

Jump to bottom

Allow adding custom classes to header, content, footer, etc. #1441

Merged
merged 1 commit into from Mar 11, 2019
Merged

Allow adding custom classes to header, content, footer, etc. #1441

merged 1 commit into from Mar 11, 2019

Conversation

limonte
Copy link
Member

@limonte limonte commented Mar 9, 2019 •
edited

Closes #1440

Swal.fire({
  ...
  customClass: {
    container: 'container-class',
    popup: 'popup-class',
    header: 'header-class',
    title: 'header-class',
    closeButton: 'close-button-class',
    content: 'content-class',
    input: 'input-class',
    actions: 'actions-class',
    footer: 'footer-class'
  }
  ...
}

(API docs sweetalert2/sweetalert2.github.io#65)

Also, customContainerClass, confirmButtonClass, cancelButtonClass, imageClass, inputClass are deprecated (sweetalert2/sweetalert2.github.io#66)

Alos, expose Swal.getHeader() (API docs sweetalert2/sweetalert2.github.io#64)

Also, in scope of this PR there's a small improvement of the deprecation warning:

Before:

image

After:

image

@limonte limonte requested a review from gverni March 9, 2019 14:23
@limonte limonte changed the title feat(api): allow adding custom classes to header, content, actions, footer Allow adding custom classes to header, content, actions, footer Mar 9, 2019
@coveralls
Copy link

coveralls commented Mar 9, 2019 •
edited

Pull Request Test Coverage Report for Build 4700

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 27 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.5%) to 91.193%
Files with Coverage Reduction New Missed Lines %
dist/sweetalert2.js 27 91.19%
Totals Coverage Status
Change from base Build 4675: 0.5%
Covered Lines: 1166
Relevant Lines: 1231
💛 - Coveralls

This was referenced Mar 9, 2019
Closed
Closed
@limonte limonte changed the title Allow adding custom classes to header, content, actions, footer Allow adding custom classes to header, content, footer, etc. Mar 9, 2019
@limonte limonte force-pushed the feat/customClass-object branch 3 times, most recently from 2875b44 to 6807b15 Compare March 9, 2019 21:32
@limonte limonte mentioned this pull request Mar 9, 2019
Closed
@limonte
Copy link
Member Author

limonte commented Mar 10, 2019

@gverni the PR is ready for review 🚀

@gverni
Copy link
Contributor

gverni commented Mar 10, 2019 •
edited

Thanks @limonte, i'm on it...

gverni
gverni suggested changes Mar 10, 2019
View reviewed changes
Copy link
Contributor

@gverni gverni left a comment

Choose a reason for hiding this comment

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

Also, what's the rationale behind leaving confirmButtonClass, cancelButtonClass, imageClass out of customClass? Shouldn't they be added as well?

src/instanceMethods/_main.js
@@ -377,6 +377,9 @@ export function _main (userParams) {
if (innerParams.inputClass) {
dom.addClass(inputContainer, innerParams.inputClass)
}
if (innerParams.customClass) {
Copy link
Contributor

Choose a reason for hiding this comment

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

For performance reasons, this should be:

if (innerParams.customClass.input)

If you set a breakpoint inside the if you will see that is called 5 times (one for each input types) even if customClass.input it's undefined. I think we should avoid that...

Copy link
Member Author

Choose a reason for hiding this comment

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

the performance difference is almost nothing here, what you're suggesting will cause this edge case to fail:

image

sweetalert2.d.ts
@@ -452,12 +464,30 @@ declare module 'sweetalert2' {

/**
* A custom CSS class for the modal.
* If a string value is provided, the classname will be applied to the popup.
Copy link
Contributor

Choose a reason for hiding this comment

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

It works also with an array of strings. Do you want to add that?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@limonte
Copy link
Member Author

limonte commented Mar 10, 2019 •
edited

Also, what's the rationale behind leaving confirmButtonClass, cancelButtonClass, imageClass out of customClass? Shouldn't they be added as well?

Good point! Done.

gverni
gverni suggested changes Mar 11, 2019
View reviewed changes
Copy link
Contributor

@gverni gverni left a comment

Choose a reason for hiding this comment

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

Apart for a small change (sorry again!!) looks good to me @limonte!

sweetalert2.d.ts Outdated Show resolved Hide resolved
@gverni
Copy link
Contributor

gverni commented Mar 11, 2019

Thanks @limonte, for me it's good now. I'll create some more example at https://github.com/sweetalert2/sweetalert2-examples and update the side bar one.

@limonte limonte merged commit 4381bae into master Mar 11, 2019
@limonte limonte deleted the feat/customClass-object branch March 11, 2019 13:34
limonte pushed a commit that referenced this pull request Mar 11, 2019
@semantic-release-bot
chore(release): 8.3.0 [skip ci]
d6ca4a0 
# [8.3.0](v8.2.6...v8.3.0) (2019-03-11)

### Bug Fixes

* remove excessive isVisible check for buttons, support Jest testing enviroment ([#1439](#1439)) ([42ef213](42ef213))

### Features

* **api:** allow adding custom classes to header, content, footer, etc. ([#1441](#1441)) ([4381bae](4381bae))
@limonte
Copy link
Member Author

limonte commented Mar 11, 2019

🎉 This PR is included in version 8.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gverni gverni gverni approved these changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add ability to set custom classes for different parts (container, header, content, footer, etc.)
3 participants
@limonte @coveralls @gverni