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

feat(toast): add header and custom toast buttons #17147

Merged
merged 41 commits into from Apr 11, 2019

Conversation

Projects
None yet
6 participants
@zakton5
Copy link
Contributor

commented Jan 17, 2019

Short description of what this resolves:

Allows more complex toasts to be utilized by allowing a header and custom buttons as discussed here: #16791

With this new feature, is showCloseButton and closeButtonText needed? I would opt for removing these properties entirely in favor of

const toast = await this.toastController.create({
      header: 'Toast header',
      message: 'Click to Close',
      position: 'top',
      buttons: [
        {
          side: 'end',
          text: 'Close',
          role: 'cancel'
        }
      ]
    });

Changes proposed in this pull request:

  • Add header property to toast options
  • Add buttons property to toast options

Ionic Version:

Fixes: #16791, possibly #16268, #16237

@zakton5 zakton5 changed the title Feat toast actions feat(toast): add header and custom toast buttons Jan 17, 2019

@paulstelzer

This comment has been minimized.

Copy link
Collaborator

commented Jan 17, 2019

Thanks a lot :) I have not checked it yet, but seems good!

Zach Keeton and others added some commits Jan 18, 2019

Zach Keeton
Merge pull request #1 from ionic-team/master
update to latest 1-22-19
Merge pull request #2 from ionic-team/master
Update master 2-1-19
@simonhaenisch

This comment has been minimized.

Copy link
Contributor

commented Feb 15, 2019

With this new feature, is showCloseButton and closeButtonText needed? I would opt for removing these properties entirely

Now that v4 is officially out, I guess you should keep them, transform and push into the buttons array if set, and add a note that they are deprecated. Introducing a breaking change might not be the best choice at the moment. The props can be dropped once a v5 release is close I guess.

brandyscarney and others added some commits Feb 15, 2019

Merge pull request #3 from ionic-team/master
Update to latest 2-16-19
@zakton5

This comment has been minimized.

Copy link
Contributor Author

commented Feb 16, 2019

@simonhaenisch @brandyscarney I've implemented your suggestion. Let me know what you think.

Zach Keeton and others added some commits Feb 18, 2019

liamdebeasi and others added some commits Mar 29, 2019

fix(toast): remove the content classes
adds the padding to content no matter if buttons are there
docs(toast): update usage section to use latest
remove deprecated button properties
@brandyscarney

This comment has been minimized.

Copy link
Member

commented Apr 10, 2019

This is so great! Sorry it took me so long to look at this. I made some minor changes:

  • Remove the classes that only added padding to the content based on the presence of buttons - this caused a bug where there was no padding on toast messages without buttons
  • Default the side to "end" if one isn't provided
  • Added a buttons test showing the old way (close props), new way (buttons array), and multiple button examples
  • Updated the usage examples (some were still using slot, react didn't have usage for buttons)
  • Adds the button role class to the button, so if the button is a cancel button it will default to the lighter (almost white) color

Unless there are any complaints, this looks good to merge to me. @liamdebeasi did you want to take another look at this?

Here are some screenshots of latest:

Two Buttons Multiple Buttons
localhost_3333_src_components_toast_test_buttons(Galaxy S5) (2) localhost_3333_src_components_toast_test_buttons(Galaxy S5) (3)
localhost_3333_src_components_toast_test_buttons(iPhone 6_7_8 Plus) (1) localhost_3333_src_components_toast_test_buttons(iPhone 6_7_8 Plus) (2)
@liamdebeasi
Copy link
Member

left a comment

Great job! 🎉

fix(toast): add hover effect to the toast buttons and pointer cursor
- adds a background color that is a transparent layer of the text color in md
- adds opacity to the button in ios
@brandyscarney

This comment has been minimized.

Copy link
Member

commented Apr 10, 2019

Okay pushed a few more changes:

  • Added a background color to the button in MD on hover (this is a transparent version of the text color)
  • Added opacity to the buttons in iOS on hover
  • Added margin end for MD buttons on the end side, and margin start for the start side

The only other thing I can find is the icon-only buttons should be rounded on hover/ripple but it could possibly be pushed to a separate PR?

@brandyscarney

This comment has been minimized.

Copy link
Member

commented Apr 10, 2019

Okay last change sorry. 😂 I updated the icon-only buttons so they have the 50% border-radius and will be rounded on hover. Good to go now!

Screen Shot 2019-04-10 at 5 00 57 PM

@zakton5

This comment has been minimized.

Copy link
Contributor Author

commented Apr 11, 2019

@brandyscarney Looks great! Nice changes. It'll be nice to finally use these haha

@brandyscarney brandyscarney merged commit 6e1a8f1 into ionic-team:master Apr 11, 2019

1 check passed

build Workflow: build
Details
@brandyscarney

This comment has been minimized.

Copy link
Member

commented Apr 11, 2019

Yay! Merged. 🎉

@simonhaenisch I tried adding us as co-authors but for some reason it didn't work. I actually sent a support message to GitHub about this as I've used the same syntax before without any issues and I'm curious why it didn't work.

@simonhaenisch

This comment has been minimized.

Copy link
Contributor

commented Apr 11, 2019

Thanks @brandyscarney, looking forward to try this out soon.

I'm pretty sure you're supposed to have two blank lines before the Co-authored-by comment(s), but I think I also had a case where I did only one and it worked anyway.

@brandyscarney

This comment has been minimized.

Copy link
Member

commented Apr 11, 2019

@simonhaenisch Yeah I'm not sure what's going on. The last time I did it was similar and it worked: a7f1f4d

Maybe adding the fixes after broke it? 🤷‍♀️

@zakton5 zakton5 deleted the zakton5:feat-toast-actions branch Apr 11, 2019

@simonhaenisch

This comment has been minimized.

Copy link
Contributor

commented Apr 11, 2019

Hm actually I think the issue might be that the "closes ..." comments are below the Co-authored-by comments... might have to be the last line(s) of the message? BTW this is where I read about needing to have two blank lines: https://help.github.com/en/articles/creating-a-commit-with-multiple-authors.

Thanks for trying, however I didn't do all too much here anyway ;)

@brandyscarney

This comment has been minimized.

Copy link
Member

commented Apr 11, 2019

Yeah, maybe. I didn't read it that well. 😂 I'm not upset to not be added either but I still want to know what I did wrong. 😭 Maybe they'll message me back, haha we'll see.

@rafalschmidt97

This comment has been minimized.

Copy link

commented May 3, 2019

Probably after this PR buttons always use primary colour (with icons and just closeButtonText). I didn't change anything in my project - just updated to the newest version. Should I add some variables into "/** Ionic CSS Variables **/" ?

IMG_2659
IMG_2658

Also please update docs to this pr 🚀

@paulstelzer

This comment has been minimized.

Copy link
Collaborator

commented May 4, 2019

@rafalschmidt97 this should be fixed with PR #18133 I am waiting for that PR merged, too ;)

Kiku-git added a commit to Kiku-git/ionic that referenced this pull request May 16, 2019

feat(toast): add header and additional custom toast buttons (ionic-te…
…am#17147)

Adds a `header` and `buttons` property to toast. This allows for a toast header to be passed and multiple buttons including action buttons and icon only buttons which matches the Material Design spec. Adds hover states to the button to match the spec. Updates usage section to recommend the new way of passing a close button using the buttons array and `cancel` role. If a button is passed using the cancel role default the color to match the spec. Buttons will default to the `end` side but have the option of being placed on the `start` side.

Co-authored-by: Simon Hänisch <simonhaenisch@users.noreply.github.com>
Co-authored-by: Brandy Carney <brandy@ionic.io>

closes ionic-team#16791 closes ionic-team#16237 closes ionic-team#17611
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.