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

RTL menu support #3200

Merged
merged 2 commits into from Dec 7, 2018

Conversation

Projects
None yet
4 participants
@karimMourra
Contributor

karimMourra commented Dec 5, 2018

This PR will...

  • prefix the playback rate with the 'x' if the 'playbackRate' localization field is RTL
    -- i.e. x1 vs 1x.
  • fix a bug where the RTL localization of auto quality would be placed in between the rate and the 'p' (i.e. 420<rtl auto>p instead of 420p <rtl auto> )

Why is this Pull Request needed?

  • RTL playback rate display should be prefixed by the x
  • the rtl auto quality localization should not be place in between the number and the p

Are there any points in the code the reviewer needs to double check?

n/a

Are there any Pull Requests open in other repos which need to be merged with this?

n/a

Addresses Issue(s):

JW8-2432

karimJWP added some commits Dec 5, 2018

karimJWP karimJWP

@karimMourra karimMourra requested review from radium-v and pajong Dec 5, 2018

@johnBartos

This comment has been minimized.

Member

johnBartos commented Dec 5, 2018

Warnings
⚠️

🛠 There are modified src files, but no test changes. Add tests if you're able to.

⚠️

🗿 Set a milestone. It should be the ticket's fix version in JIRA.

Generated by 🚫 dangerJS

@jwplayer-robot

This comment has been minimized.

jwplayer-robot commented Dec 5, 2018

MULTI Build for commit b9bf7de passed.
🏗 jwplayer build SUCCESS
🏗 jwplayer browserstack tests SUCCESS
🏗 jwplayer-commercial build SUCCESS
🏗 jwplayer-commercial browserstack tests SUCCESS
🥒 Automated Tests SUCCESS
🍆 Manual Tests
📺 Views

@johnBartos

LGTM but I think @radium-v should approve before merging too

@karimMourra karimMourra merged commit 110e824 into master Dec 7, 2018

4 checks passed

Danger ⚠️ Danger found some issues. Don't worry, everything is fixable.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
jw7-pr-multi-opensource Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment