Skip to content

Ux styling #46

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

Merged
merged 16 commits into from
Dec 14, 2018
Merged

Ux styling #46

merged 16 commits into from
Dec 14, 2018

Conversation

jamesdools
Copy link
Contributor

No description provided.

@pietrop pietrop self-requested a review December 11, 2018 21:46
package.json Outdated
@@ -76,6 +76,7 @@
"babel-polyfill": "^6.26.0",
"draft-js": "^0.10.5",
"mousetrap": "1.5.2",
"node-sass": "^4.10.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

👋 is this in use? I mean are we using sass or css modules? or both?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch - was intending to use it - but isn't in use at the moment.

@pietrop
Copy link
Contributor

pietrop commented Dec 13, 2018

some initial thoughts


recap of functionalities

Just a quick re-cap of the separation of functionalities into main control bar, settings panel shortcut tips.

Main control bar

  • rollback
  • rewind
  • fast forward - see issue #47
  • timecode display
  • Jump to time functionality - by clicking on timecode - possibly with tool tip to flag option?
  • duration display
  • playback speed indication x1
  • playback speed control - dropdown to change based on presets (or range with bars?)
  • reset playback speed ( Eg on playback speed indication + tooltip?) might not be needed if it's a drop down.

Settings

  • roll back customisable amount
  • pause while typing toggle
  • scroll sync toggle
  • adjust time-code offset

cheat sheets

  • Keyboard shortcut cheat sheet
  • How does this work - cheat sheet

Raised as separate issue to be addressed at later stage - #48


component height

Was trying to figure out why the component takes the height of window rather then parent element ?

  • Eg in demo page it covers demo controls
  • This might make it harder to integrate in other apps. - altho we are looking at CSS encapsulation into another ticket, it might be good to sort out the patent height thing now if it is a quick fix? As it might also work better with the demo page?

might be enough to change the position of the MediaPlayer to be position: fixed; ?
I did a quick test and it seemed to do the trick

screen shot 2018-12-13 at 12 58 55


issue with width of the control panel.

see screenshot.

screen shot 2018-12-13 at 11 15 35

to reproduce

  • npm start
  • go to http://localhost:3006/
  • click load demo
  • should look same as screenshot (?)
  • sometimes re-adjusting window width makes it stretch to full with other times not

Transcript name

The current designs have the transcript name above the controls. And for now it's just taking the URL.
If we want to keep that, I suggest we pass an optional transcript title/name to the parent component and show that if present.
At this stage I would not make this editable. as it becomes one more thing to keep track of.
And often the transcript STT json will not have the title info in it.

The idea being to keep a simple interface for the main TranscriptEditor. - but yeah something to thing about...


height of paragraph

seems like some paragraphs might have an internal scroll within the main scroll of the text? (if that makes sense?) - see screenshot

screen shot 2018-12-13 at 12 57 58


that's all I got for now, will have another look later

@jamesdools jamesdools merged commit 2bbcc3a into master Dec 14, 2018
@jamesdools jamesdools deleted the ux-styling branch December 14, 2018 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants