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

Subs Caps Button isn't sized fully in v6 #4547

Closed
gkatsev opened this issue Aug 7, 2017 · 6 comments
Closed

Subs Caps Button isn't sized fully in v6 #4547

gkatsev opened this issue Aug 7, 2017 · 6 comments

Comments

@gkatsev
Copy link
Member

gkatsev commented Aug 7, 2017

Description

The new Subs Caps Button in v6 has full functionality, however, when you click on it, you can see that the focus outline is sized to only the font-icon rather than the entire button itself.
screen shot 2017-08-07 at 17 17 37
Where as it should look like so:
screen shot 2017-08-07 at 17 20 14

Steps to reproduce

  1. Create a player
  2. Add some captions or subtitles
  3. Play the video
  4. Click on the subs caps button.

Results

Expected

Expected to outline the entire button
screen shot 2017-08-07 at 17 20 14

Actual

The outline only covers the font icon itself
screen shot 2017-08-07 at 17 17 37

Proposed solution

In the button.scss file, add a new selector that targets .vjs-button class inside of a .vjs-control class and sizes it to be 100%

.vjs-control .vjs-button {
  width: 100%;
  height: 100%;
}

To rebuild the styles locally, you can then run grunt skin or npm run grunt -- skin.

@agiridharan
Copy link

HI I would like to help with this bug issue! Is there anyone else working on it right now?

@gkatsev
Copy link
Member Author

gkatsev commented Aug 7, 2017

@agiridharan feel free to grab this. No one is working on it as far as I know. Thanks!

@agiridharan
Copy link

Hi @gkatsev I'm not understanding how to setup the video.js. I have run the build command, but I'm stuck on what I should do after that.

@gkatsev
Copy link
Member Author

gkatsev commented Aug 9, 2017

@agiridharan hey, sorry it took so long for me to get back to you. Unfortunately, seems like someone already sniped this issue :( ( #4548 ). If you're still interested in trying to get this set up and working, I'd be happy to help you out. We have general docs about contributing but essentially, the best way to do it is by clone the repo, run npm install and then run npm start. Then you can do to localhost:9999/sandbox/. You'd also need to move sandbox/index.html.example to sandbox/index.html.

@gkatsev
Copy link
Member Author

gkatsev commented Aug 9, 2017

Also, I'd hope to get more first-timers-only issues soon.

@mugendin
Copy link

Hi @gkatsev, i would like to help.

mugendin pushed a commit to mugendin/video.js that referenced this issue Aug 10, 2017
add as proposed 100% width and height and reorder css properties according to https://css-tricks.com/poll-results-how-do-you-order-your-css-properties/ fixies issue videojs#4547
atefBB pushed a commit to atefBB/video.js that referenced this issue Aug 16, 2017
…deojs#4548)

Add a width and height property of 100% to `.vjs-control .vjs-button` selector.

Fixes videojs#4547.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants