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

fix(toast): allow to change position start/end of toast #17961

Merged
merged 24 commits into from Apr 16, 2019

Conversation

Projects
None yet
3 participants
@entom
Copy link
Contributor

commented Apr 4, 2019

Short description of what this resolves:

This allows user to set start and/or end position of toast-wrapper by using --start, --end.

Changes proposed in this pull request:

Ionic Version:
4.2.0
Fixes: #17854

@ionitron-bot ionitron-bot bot added the package: core label Apr 4, 2019

entom and others added some commits Apr 4, 2019

@liamdebeasi
Copy link
Member

left a comment

This looks great! Can you just add some tests to src/components/toast/test/basic/index.html that show this working?

Thanks!

entom added some commits Apr 5, 2019

@entom

This comment has been minimized.

Copy link
Contributor Author

commented Apr 5, 2019

Ok, no problem. I have created some tests.

entom added some commits Apr 7, 2019

@liamdebeasi

This comment has been minimized.

Copy link
Member

commented Apr 11, 2019

Hey there!

Sorry for the delay in reviewing this. We had an update to toast that we were trying to get merged in, and unfortunately there were some merge conflicts created as a result.

Would you be able to resolve the conflicts in your PR? After that I can take another look.

Thanks! 🙂

entom added some commits Apr 11, 2019

@entom

This comment has been minimized.

Copy link
Contributor Author

commented Apr 11, 2019

Ok, no problem. Thanks for response. I have resolved conflicts and merged changes.

@brandyscarney brandyscarney requested a review from liamdebeasi Apr 11, 2019

liamdebeasi added some commits Apr 16, 2019

@liamdebeasi
Copy link
Member

left a comment

Great work! 👍

@liamdebeasi liamdebeasi changed the title fix(toast): allow to change position left/right of toast fix(toast): allow to change position start/end of toast Apr 16, 2019

@brandyscarney brandyscarney self-requested a review Apr 16, 2019

brandyscarney added some commits Apr 16, 2019

@brandyscarney

This comment has been minimized.

Copy link
Member

commented Apr 16, 2019

Thanks for the PR! We made some changes that I just wanted to make aware, the big one being renaming the following:

left -> start
right -> end

The main reason for this is because in RTL applications the left is actually the right, and the right is the left so in many places we are using start and end now for RTL support. 🙂

I made a couple changes to the implementation but this is good to go! We can merge it once the build passes.

@brandyscarney brandyscarney merged commit 07e739a into ionic-team:master Apr 16, 2019

1 check passed

build Workflow: build
Details
@brandyscarney

This comment has been minimized.

Copy link
Member

commented Apr 16, 2019

Merged, thank you!!

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.