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

Player - Fix initial options merge #43

Merged
merged 1 commit into from
Jan 15, 2021

Conversation

joaopaulovieira
Copy link
Member

@joaopaulovieira joaopaulovieira commented Nov 1, 2020

Summary

The defaultOptions.playback being overwritten when the received options on Clappr init have the playback options too. The expected behavior is the merge between those two options.

This PR applies the expected behavior using the Zepto $.extend() method with the option for a deep merge approach.

Changes

  • Applies the deep merge config on every $.extend call;

How to test

  • Play one media;
  • Print the value of player.options.playback on developr tools console tab;
    • The object should contain the property recycleVideo: true and controls: true.

A picture tells a thousand words

Before this PR

Screen Shot 2020-11-01 at 02 05 02

After this PR

Screen Shot 2020-11-01 at 02 04 21

@joaopaulovieira joaopaulovieira added the bug Something isn't working label Nov 1, 2020
@joaopaulovieira joaopaulovieira changed the title Player - Fix intial options merge Player - Fix initial options merge Nov 1, 2020
@joaopaulovieira joaopaulovieira force-pushed the fix-intial-options-merge branch 2 times, most recently from 7b00f50 to 4f7421e Compare November 19, 2020 17:49
@joaopaulovieira joaopaulovieira force-pushed the fix-intial-options-merge branch from 4f7421e to 7d9eb28 Compare January 14, 2021 17:56
@joaopaulovieira joaopaulovieira merged commit 7eeb065 into master Jan 15, 2021
@joaopaulovieira joaopaulovieira deleted the fix-intial-options-merge branch January 15, 2021 01:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants