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: Replace Object.values with ponyfill #8267

Merged
merged 9 commits into from
May 11, 2023
Merged

Conversation

dzianis-dashkevich
Copy link
Contributor

@dzianis-dashkevich dzianis-dashkevich commented May 8, 2023

Description

Context: #8266

Specific Changes proposed

  • Add videojs.obj.values util.
  • Replace Object.values usage.

Note

  • Currently, BrowserStack Chrome 53 setup is failing. We should set up a proper Chrome 53 separate test environment.
  • Firefox 64 version was unpinned in this PR.

Requirements Checklist

  • Feature implemented / Bug fixed
  • If necessary, more likely in a feature request than a bug fix
    • Change has been verified in an actual browser (Chrome, Firefox, IE)
    • Unit Tests updated or fixed
    • Docs/guides updated
    • Example created (starter template on JSBin)
  • Reviewed by Two Core Contributors

… (specifically for chrome 53 which is aligned with WebOS 4)
@codecov
Copy link

codecov bot commented May 8, 2023

Codecov Report

Merging #8267 (d189a6c) into main (1491d71) will increase coverage by 82.36%.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##           main    #8267       +/-   ##
=========================================
+ Coverage      0   82.36%   +82.36%     
=========================================
  Files         0      112      +112     
  Lines         0     7483     +7483     
  Branches      0     1804     +1804     
=========================================
+ Hits          0     6163     +6163     
- Misses        0     1320     +1320     
Impacted Files Coverage Δ
src/js/title-bar.js 100.00% <ø> (ø)
src/js/utils/obj.js 75.86% <100.00%> (ø)

... and 110 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@jonybekov
Copy link

Is it going to be released as v7 patch or v8?

@dzianis-dashkevich dzianis-dashkevich changed the title chore: add usage polyfills from core-js for browser-list old browsers… fix: Replace Object.values with polyfill. Pin Chrome 53 in BrowserStack May 9, 2023
@dzianis-dashkevich
Copy link
Contributor Author

Is it going to be released as v7 patch or v8?

@jonybekov, I think only v8 since I don't see any Object.values usage in v7

@dzianis-dashkevich dzianis-dashkevich self-assigned this May 9, 2023
@gkatsev gkatsev changed the title fix: Replace Object.values with polyfill. Pin Chrome 53 in BrowserStack fix: Replace Object.values with ponyfill May 11, 2023
@gkatsev gkatsev merged commit 866ef24 into main May 11, 2023
@gkatsev gkatsev deleted the chrome-53-web-os-4-support branch May 11, 2023 22:57
edirub pushed a commit to edirub/video.js that referenced this pull request Jun 8, 2023
Unpin Firefox in BrowserStack tests, which fixes test failures as well.

Fixes videojs#8266.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants