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

Add bgcolor and adopt the height of the video scrubber #287

Merged
merged 1 commit into from
Sep 18, 2020

Conversation

ChrisEdS
Copy link
Contributor

@ChrisEdS ChrisEdS commented Aug 14, 2020

With this PR I added a background color to the scrubber. Especially with long videos it was not obvious at the beginning that there is a scrubber.

The height of the scrubber was increased by 5 pixels. @PVince81 can you do a re-test with these changes?

Fixes: #26

Screenshot

Before:
image

After:
image

Manually tested in Chrome browser

width: 100%;
transform: translateY(-100%);
transition: height .25s;
background: rgb(0 0 0 / 0.5);
Copy link
Contributor

Choose a reason for hiding this comment

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

I get a compile error with make dist, it says:

    Module build failed (from ./node_modules/sass-loader/dist/cjs.js):
    SassError: Function rgb is missing argument $green.
            on line 36 of /srv/www/htdocs/owncloud/apps-external/files_mediaviewer/src/styles/default.scss
    >>          background: rgb(0 0 0 / 0.5);

I think you should add commas

Copy link
Contributor

Choose a reason for hiding this comment

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

I've fixed it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

❤️

Copy link
Contributor

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

👍 looks good :-)

@ChrisEdS ChrisEdS merged commit ee78e69 into master Sep 18, 2020
@delete-merged-branch delete-merged-branch bot deleted the fix-scrubber branch September 18, 2020 07:35
@jnweiger jnweiger mentioned this pull request Jan 10, 2021
31 tasks
@jnweiger
Copy link
Contributor

Confirmed fixed in 1.0.4

Tested with google chrome and firefox.

The scrubber background is much darker than in the above screenshots, but easily recognizable
image

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

Successfully merging this pull request may close these issues.

Video scrubber not intuitive
3 participants