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

fix(item-sliding): hide opposite side options #17986

Merged
merged 7 commits into from Apr 15, 2019
Merged

fix(item-sliding): hide opposite side options #17986

merged 7 commits into from Apr 15, 2019

Conversation

masimplo
Copy link
Contributor

@masimplo masimplo commented Apr 6, 2019 •
edited

Short description of what this resolves:

This resolves an issue where both ion-item-options are show when sliding an ion-item-sliding item.

Changes proposed in this pull request:

  • adds a css rule to hide the opposite item-options when sliding item is open
  • adds an item in basic tests to demonstrate the issue and fix

Ionic Version:

Fixes: #17822

Video demonstrating issue and fix:
video

@ionitron-bot ionitron-bot bot added the package: core @ionic/core package label Apr 6, 2019
@masimplo masimplo mentioned this pull request Apr 6, 2019
Closed
liamdebeasi
liamdebeasi suggested changes Apr 11, 2019
View reviewed changes
Copy link
Contributor

@liamdebeasi liamdebeasi left a comment

Choose a reason for hiding this comment

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

Hey there,

Thanks for the PR -- this is looking great!

One small thing I'm noticing is if you drag an item-sliding all the way over to one side, you can see the last bit of one of the buttons (if there are multiple):

Screen Shot 2019-04-11 at 11 51 51 AM

If you change the 90% in item-sliding/item-sliding.scss:36 and item-sliding/item-sliding.scss:54 to 100% that should fix the issue.

@masimplo
Copy link
Contributor Author

Hi,
thanks for looking into this. You are correct, it was a bit tricky to reproduce as it does not happen in the emulator but on desktop. Nice catch!
Just pushed the proposed fix, which works like a charm.

liamdebeasi
liamdebeasi approved these changes Apr 11, 2019
View reviewed changes
Copy link
Contributor

@liamdebeasi liamdebeasi left a comment

Choose a reason for hiding this comment

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

Great job! 🎉

@liamdebeasi
Copy link
Contributor

Hey there! Per @brandyscarney's comment #17822 (comment) I've updated the CSS a little bit to fix the issue. Looks like the visibility: hidden got removed during our v3->v4 transfer, so I added it back.

@brandyscarney brandyscarney self-requested a review April 11, 2019 20:16
@liamdebeasi liamdebeasi merged commit f13722c into ionic-team:master Apr 15, 2019
@liamdebeasi
Copy link
Contributor

Merged. Thank you! 🎉

@masimplo masimplo deleted the fix-sliding-buttons-visibility branch April 16, 2019 12:48
kiku-jw pushed a commit to kiku-jw/ionic that referenced this pull request May 16, 2019
@masimplo @kiku-jw
fix(item-sliding): hide opposite side options when other side is open (…
ca34d05 
…ionic-team#17986)

* fix(item-sliding): hide opposite side options

* Fix padding issue

* add visibility hidden for item options
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liamdebeasi liamdebeasi liamdebeasi approved these changes

@brandyscarney brandyscarney brandyscarney approved these changes

Assignees
No one assigned
Labels
package: core @ionic/core package
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@masimplo @liamdebeasi @brandyscarney