Skip to content
This repository was archived by the owner on May 1, 2024. It is now read-only.

Added Speed Property to mediaelement #1415

Closed
wants to merge 17 commits into from
Closed

Added Speed Property to mediaelement #1415

wants to merge 17 commits into from

Conversation

AswinPG
Copy link
Contributor

@AswinPG AswinPG commented Jun 18, 2021 •
edited by jfversluis
Loading

Description of Change

Added Speed property for Mediaelement in iOS and Android

Bugs Fixed

API Changes

PR Checklist

  • Has a linked Issue, and the Issue has been approved
  • Has tests (if omitted, state reason in description)
  • Has samples (if omitted, state reason in description)
  • Rebased on top of main at time of PR
  • Changes adhere to coding standard

@AswinPG AswinPG mentioned this pull request Jun 18, 2021
Closed
4 tasks
@jfversluis
Copy link
Member

Seems to work great on Android and iOS at least, thanks for this!

Would you be able to add this to the sample app too? Ideally some way to change the new Speed property in the MediaElement page? A slider maybe? Thanks!

@AswinPG
Copy link
Contributor Author

AswinPG commented Jun 28, 2021

@jfversluis I have added the sample in the media element page :)

@jfversluis
Copy link
Member

One last request: could you rebase it please? Or at least update it with the latest changes? I would do it for you, but I don't have the rights to your branch :)

@AswinPG
Copy link
Contributor Author

AswinPG commented Jun 28, 2021 •
edited
Loading

One last request: could you rebase it please? Or at least update it with the latest changes? I would do it for you, but I don't have the rights to your branch :)

Did it :)

AswinPG and others added 4 commits July 2, 2021 02:44
@jfversluis jfversluis enabled auto-merge (squash) July 15, 2021 14:00
@jfversluis
Copy link
Member

OK, so sorry. One last rebase and then it should merge automatically... 😇

thanhxuann-clearwave
thanhxuann-clearwave previously approved these changes Jul 16, 2021
View reviewed changes
Copy link

@thanhxuann-clearwave thanhxuann-clearwave left a comment

Choose a reason for hiding this comment

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

LGTM

@jfversluis
Copy link
Member

OK, so sorry. One last rebase and then it should merge automatically... 😇

OMG... WHY DIDN'T IT DO IT?!

jfversluis
jfversluis previously approved these changes Jul 16, 2021
View reviewed changes
@AswinPG
Copy link
Contributor Author

AswinPG commented Jul 17, 2021

OK, so sorry. One last rebase and then it should merge automatically... 😇

OMG... WHY DIDN'T IT DO IT?!

Is there something that I should do ?

@pictos
Copy link
Contributor

pictos commented Jul 17, 2021

@AswinPG looks like this branch is out-of-date, can you rebase your branch against develop and push?

auto-merge was automatically disabled July 17, 2021 16:17

Head branch was pushed to by a user without write access

@AswinPG AswinPG dismissed stale reviews from jfversluis and thanhxuann-clearwave via 4e9b0cd July 17, 2021 16:17
@AswinPG
Copy link
Contributor Author

AswinPG commented Jul 17, 2021

@AswinPG looks like this branch is out-of-date, can you rebase your branch against develop and push?

It's my first time rebasing. I don't really know if I did it right or wrong...

@pictos
Copy link
Contributor

pictos commented Jul 17, 2021

@AswinPG yeah, something went wrong... We can see based on the number of changed files

@pictos
Copy link
Contributor

pictos commented Jul 17, 2021

I see... Looks like that you made your changes against our main branch instead of the develop branch.

@pictos pictos changed the base branch from develop to main July 17, 2021 19:57
@pictos pictos changed the base branch from main to develop July 17, 2021 19:57
@pictos
Copy link
Contributor

pictos commented Jul 17, 2021

@AswinPG looks like everything is ok now, let's how it goes. I'll let @jfversluis take a final look at this since this rebase dismissed his review.

@AswinPG
Copy link
Contributor Author

AswinPG commented Jul 17, 2021

@AswinPG looks like everything is ok now, let's how it goes. I'll let @jfversluis take a final look at this since this rebase dismissed his review.

Thanks @pictos

1 similar comment
@AswinPG
Copy link
Contributor Author

AswinPG commented Jul 17, 2021

@AswinPG looks like everything is ok now, let's how it goes. I'll let @jfversluis take a final look at this since this rebase dismissed his review.

Thanks @pictos

@jfversluis jfversluis mentioned this pull request Aug 11, 2021
Merged
6 tasks
@jfversluis
Copy link
Member

Implemented through #1570 to work around asking for another rebase :)

Thanks so much @AswinPG !

@jfversluis jfversluis closed this Aug 11, 2021
@AswinPG
Copy link
Contributor Author

AswinPG commented Aug 16, 2021

Thanks @jfversluis :)

Karuuv added a commit to Karuuv/xamarin-communitytoolkit that referenced this pull request May 13, 2022
@Karuuv
Add "Speed" property info.
3bf8ab6 
Adding details about the "Speed" property added with xamarin/XamarinCommunityToolkit#1415
@Karuuv Karuuv mentioned this pull request May 13, 2022
Merged
4 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@AndreiMisiukevich AndreiMisiukevich AndreiMisiukevich left review comments

@jfversluis jfversluis jfversluis left review comments

@thanhxuann-clearwave thanhxuann-clearwave thanhxuann-clearwave left review comments

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

Successfully merging this pull request may close these issues.
5 participants
@AswinPG @jfversluis @pictos @AndreiMisiukevich @thanhxuann-clearwave