Skip to content

Display time "moves" during time update #73

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

Closed
pietrop opened this issue Jan 14, 2019 · 8 comments
Closed

Display time "moves" during time update #73

pietrop opened this issue Jan 14, 2019 · 8 comments
Labels
bug Something isn't working CSS Something to do with styling good first issue Good for newcomers help wanted Extra attention is needed

Comments

@pietrop
Copy link
Contributor

pietrop commented Jan 14, 2019

Describe the bug

To Reproduce

  • Load a transcript and click play
  • watch the display time

Expected behavior

expect it not to move

Screenshots

bug-time-display

Additional context

Might be a quick CSS fix?

@pietrop pietrop added bug Something isn't working CSS Something to do with styling help wanted Extra attention is needed labels Jan 14, 2019
@pietrop
Copy link
Contributor Author

pietrop commented Jan 16, 2019

Can’t reproduce in demo app, This might be a problem when component is used in another react app (which is where this issue was observed). Might be issue with CSS height/width of parent component(?)

@pietrop
Copy link
Contributor Author

pietrop commented Mar 15, 2019

closing this, as CSS has moved on since, but can be reopened if anyone comes across it

@pietrop pietrop closed this as completed Mar 15, 2019
@pietrop
Copy link
Contributor Author

pietrop commented Mar 22, 2019

Added the component to an app with React bootstrap and seems like the bug is not gone?
Altho it doesn't show up in the demo app.

bug-rte

@pietrop pietrop reopened this Mar 22, 2019
@Gribbs
Copy link

Gribbs commented Apr 8, 2019

I also see the same behavior when adding the component to a different App

@bevand10
Copy link
Contributor

bevand10 commented Apr 8, 2019

You need to use a monospace font to avoid movement.

@pietrop pietrop added the good first issue Good for newcomers label Apr 8, 2019
@pietrop
Copy link
Contributor Author

pietrop commented Apr 8, 2019

ok, thanks @Gribbs and @bevand10, so a fix could be to add a monospace font-family to PlayerControls.module.css such as

...
.currentTime {
   ...
  font-family: 'Lucida Console',  monospace;
}

.duration {
  ...
  font-family: 'Lucida Console',  monospace;
}

This

Screen Shot 2019-04-08 at 16 19 53

Don't want to get into fonts and UI details, but I am not sure tho how this will work, for example when adding into a parent app where we have BBC Reith fonts. Because of css specificity rules currentTime and duration might stick to mono and not pick up the Reith font, and might stand out as being of a different font.

Just thinking out loud if there's a quick fix for this (cc @jamesdools ).

pietrop pushed a commit that referenced this issue Apr 8, 2019
 in media player to stop moving while playing
@bevand10
Copy link
Contributor

bevand10 commented Apr 8, 2019

The point with timecode display is that is should always be monospace.

Even in a BBC scenario where Reith is used elsewhere, it should still be a monospace font otherwise it'll bounce around just as it does now - Reith does not have a monospace variant.

It's because, to avoid horizontal movement, the width of 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 and : must be identical.

jamesdools pushed a commit that referenced this issue Apr 8, 2019
* got storybook working

tests not working yet

* fixed tests

using jest, and removed CRA dependency, updated babel config for babel 7 and stubbed css files for jest tests for css node modules

* Added support for demo app in storybook

* fixed eslint

CRA had it's own linter internally, so added linting + dependencies

* cleaned up export scripts in package.json

* updated README

* finalised refactor

see PR description for more details

* rename demo app editor to demoTranscript

* bringing back style lint, and fixing lint in storybook config

* updated with current master AWS adapter

* linting

* fix #132 playtime displaied double

 playtime on display is double of actual total playtime

* temporary fix #73 monospace duration and current time

 in media player to stop moving while playing
@pietrop
Copy link
Contributor Author

pietrop commented Apr 8, 2019

Awesome, thanks for clarifying that @bevand10
closing this issue it has been addressed in the "monorepo refactor" branch #130

@pietrop pietrop closed this as completed Apr 8, 2019
pietrop added a commit that referenced this issue Apr 8, 2019
* Task: added lerna

* WIP: storybook conversion

* mend

* WIP: adding MediaPlayer

* move TimedTextEditor, TranscriptEditor and adapters util to packages (#128)

* moved TimedTextEditor and TranscriptEditor to packages

also created stories, and package.json for each, but can't test them in storybook coz they have dependencies on adapters in Util folder

* moved Util and demdemo app

* got storybook working

* added demo app to storybook

* mend

* Fix: commenting out demo

* Changed repo packages folder structure (#129)

* cleaned up adapters

* changed folder structure

* fixed timecode converter duplice module

* made all packages private except for TranscriptEditor

* working

* "Monorepo" refactor spike remove lerna (#135)

* got storybook working

tests not working yet

* fixed tests

using jest, and removed CRA dependency, updated babel config for babel 7 and stubbed css files for jest tests for css node modules

* Added support for demo app in storybook

* fixed eslint

CRA had it's own linter internally, so added linting + dependencies

* cleaned up export scripts in package.json

* updated README

* finalised refactor

see PR description for more details

* rename demo app editor to demoTranscript

* bringing back style lint, and fixing lint in storybook config

* updated with current master AWS adapter

* linting

* fix #132 playtime displaied double

 playtime on display is double of actual total playtime

* temporary fix #73 monospace duration and current time

 in media player to stop moving while playing

* Feature: Added custom css loading to storybook (#136)

* Resolved conflict iwth AWS adapter
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working CSS Something to do with styling good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants