Skip to content

move TimedTextEditor, TranscriptEditor and adapters util to packages #128

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

Conversation

pietrop
Copy link
Contributor

@pietrop pietrop commented Apr 5, 2019

Is your Pull Request request related to another issue in this repository ?

A spike on branch monorepo-refactor

Describe what the PR does

  • move TimedTextEditor and TranscriptEditor to packages
  • as well as adapters dependencies from Util

State whether the PR is ready for review or whether it needs extra work
still working on it

Additional context
🚀

also created stories, and package.json for each, but can't test them in storybook coz they have dependencies on adapters in Util folder
@pietrop pietrop changed the title moved TimedTextEditor and TranscriptEditor to packages move TimedTextEditor, TranscriptEditor and adapters util to packages Apr 5, 2019
@pietrop pietrop marked this pull request as ready for review April 5, 2019 12:07
@pietrop
Copy link
Contributor Author

pietrop commented Apr 5, 2019

Got the storybook working! based on @chrishutchinson PR #122

There's still stuff to do, but as short lived spike, I think it be good to review it and merge it to the mono repo branch, before doing another spike with some more tweaks.

so far done

  • Moved everything in src/lib into packages
  • config lerna so that it can pick up nested folder structure inside of packages
  • demo page - The advantage of keeping. refactoring demo page, is that you can also have a direct link to full screen version, eg publishing in github pages http://localhost:6006/iframe.html?id=demo--default

Still to do, for another spike, PR

  • folder structure for packages folder - see this example
  • re-think the naming of the modules, eg is this a good naming convention @bbc/react-transcript-editor-timecode-converter as a workaround to the name spacing? or is it not very good? do we even need to name space them? we could just decide which once are public eg they are reusable by themselves, so they are published to npm , and in which case what is a sensible standalone name, and which once are private, meaning used only within the component) - this could be in more then one pass.
  • figure out how to publish to npm only certain packages with lerna.
  • figure out how to publish storybook to github pages
  • add storybook plugins

Pietro Passarelli - News Labs added 2 commits April 5, 2019 13:21
@pietrop
Copy link
Contributor Author

pietrop commented Apr 5, 2019

some quick lerna tips, that we should add to docs notes

use npx to call local version of lerna, as it's installed as dev dependency

npx lerna clean

will ask for confirmation, and removes all the symlink and node_modules that are connecting the various packages in the mono repo.

npx lerna ls

lists all the modules it recognises in packages folder and other folders specified in lerna.json config. useful to see if it is picking up everything you expect.

npx lerna bootstrap

links all the packages together as needed.

if that works, you should then be ready to do

npm run storybook 

should take you to http://localhost:6006 where you should see the storybook. unless any errors are flagged in the terminal.

@jamesdools jamesdools merged commit 9e28477 into monorepo-refactor Apr 5, 2019
@jamesdools jamesdools deleted the monorepo-refactor-spike-timed-text-and-transcript-editor branch April 5, 2019 14:00
pietrop added a commit that referenced this pull request 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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants