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: error message should not be localized in the player class #7776

Merged
merged 1 commit into from
May 31, 2022

Conversation

gjanblaszczyk
Copy link
Member

Description

The "not supported message" should not be localized in the player class because an "Error Display" class localizes all error messages before showing them to the user.
https://github.com/videojs/video.js/blob/main/src/js/error-display.js#L50

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

@codecov
Copy link

codecov bot commented May 24, 2022

Codecov Report

Merging #7776 (8e9779d) into main (7e2b9ec) will increase coverage by 0.01%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##             main    #7776      +/-   ##
==========================================
+ Coverage   80.91%   80.92%   +0.01%     
==========================================
  Files         116      116              
  Lines        7457     7461       +4     
  Branches     1807     1811       +4     
==========================================
+ Hits         6034     6038       +4     
  Misses       1423     1423              
Impacted Files Coverage Δ
src/js/player.js 88.37% <50.00%> (ø)
src/js/resize-manager.js 77.08% <0.00%> (ø)
src/js/component.js 94.19% <0.00%> (+0.02%) ⬆️
src/js/video.js 93.85% <0.00%> (+0.10%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7e2b9ec...8e9779d. Read the comment docs.

Copy link
Member

@misteroneill misteroneill left a comment

Choose a reason for hiding this comment

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

Nice catch, thanks @gjanblaszczyk!

I don't know what codecov is complaining about here, but this is fine.

@misteroneill misteroneill added the patch This PR can be added to a patch release. label May 25, 2022
@misteroneill misteroneill changed the title Fix: error message should not be localized in the player class fix: error message should not be localized in the player class May 25, 2022
@mister-ben
Copy link
Contributor

We should deprecate the notSupportedMessage option in 8 too.

@misteroneill misteroneill merged commit 75ea699 into videojs:main May 31, 2022
@gjanblaszczyk gjanblaszczyk deleted the fix/localized-error-message branch May 31, 2022 20:10
edirub pushed a commit to edirub/video.js that referenced this pull request Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch This PR can be added to a patch release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants