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.

Sign up for GitHub

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

Jump to bottom

Fix: enable utilities on video component #835

Merged
merged 7 commits into from
Aug 16, 2018
Merged

Fix: enable utilities on video component #835

merged 7 commits into from
Aug 16, 2018

Conversation

mikemai2awesome
Copy link
Collaborator

Jira

http://vjira2:8080/browse/BDS-336

Summary

All utility classes are enabled for the video component.

Details

The video component sometimes has a ratio object wrapping the video tag, this causes certain utility classes to not work. The updates addresses that scenario. Utility classes will now go on the ratio object while the video classes are still passed to the video tag.

screen shot 2018-08-13 at 11 53 56 am

How to test

Pull down the branch and test the following code on any page.

<figure>
  {% include "@bolt/video.twig" with {
    "videoId": "3861325118001",
    "accountId": "1900410236",
    "playerId": "r1CAdLzTW",
    "showMeta": true,
    "showMetaTitle": false,
    "attributes": {
      "class": "u-bolt-margin-bottom-small"
    }
  } only %}
  <figcaption>
    {% include "@bolt/text.twig" with {
      "size": "xsmall",
      "text": "EE: Transforming the Customer Journey Through 1:1 Customer Engagement (Suzanne Woolley)"
    } only %}
  </figcaption>
</figure>

Double check that the utility classes are passed to the ratio object and video classes are passed to the video tag.

@bolt-bot
Copy link
Collaborator

⚡ PR built on Travis and deployed a now preview here:

@bolt-bot
Copy link
Collaborator

⚡ Sauce Labs Test for chrome Passed!

Image of chrome test
Test Details

@bolt-bot
Copy link
Collaborator

⚡ Sauce Labs Test for internet explorer Passed!

Image of internet explorer test
Test Details

@bolt-bot
Copy link
Collaborator

⚡ Sauce Labs Test for chrome Passed!

Image of chrome test
Test Details

@bolt-bot
Copy link
Collaborator

⚡ Sauce Labs Test for internet explorer Passed!

Image of internet explorer test
Test Details

@remydenton
chore: move video class logic inside conditional (BDS-336)
5c09fc8 
A subtle change, but this improves performance (only run the
class-sorting code for ratio if necessary) and I think readability (by
making it more clear that the class sorting is only necessary if ratio
is being used).
@remydenton
fix: rework logic for separating utility classes for video with ratio
0ec92da 
(BDS-336)

- Replaces arrays and merges attributes objects and addClass/removeClass
methods.
- Most notably, allows other attributes besides class to be passed to
_video-tag when using ratio.
@remydenton remydenton mentioned this pull request Aug 14, 2018
Merged
remydenton
remydenton requested changes Aug 14, 2018
View reviewed changes
Copy link
Collaborator

@remydenton remydenton left a comment

Choose a reason for hiding this comment

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

I discovered that these updates removed the ability to pass other attributes through to the video when using the ratio object (which is included by default).

I created a "sub-feature" PR at #836. @mikemai2awesome, please review that PR and merge it into this one if everything checks out. After that, this should be ready to merge.

@remydenton
Merge pull request #836 from bolt-design-system/subfeature/bds-336-fi…
785cfef 
…x-attributes-support

Subfeature/bds 336 fix attributes support
@bolt-bot
Copy link
Collaborator

⚡ PR built on Travis and deployed a now preview here:

@bolt-bot
Copy link
Collaborator

⚡ Sauce Labs Test for chrome Passed!

Image of chrome test
Test Details

@bolt-bot
Copy link
Collaborator

⚡ Sauce Labs Test for internet explorer Passed!

Image of internet explorer test
Test Details

@bolt-bot
Copy link
Collaborator

⚡ Sauce Labs Test for chrome Passed!

Image of chrome test
Test Details

@bolt-bot
Copy link
Collaborator

⚡ Sauce Labs Test for internet explorer Passed!

Image of internet explorer test
Test Details

@remydenton remydenton merged commit 2339eac into master Aug 16, 2018
@remydenton remydenton deleted the fix/bds-336-enable-utilities-on-video branch August 16, 2018 14:01
@remydenton remydenton mentioned this pull request Aug 16, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@remydenton remydenton remydenton approved these changes

@sghoweri sghoweri Awaiting requested review from sghoweri

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

Successfully merging this pull request may close these issues.
3 participants
@mikemai2awesome @bolt-bot @remydenton