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

Develop branch - should component update refactor #182

Merged
merged 6 commits into from
Aug 12, 2019

Conversation

pietrop
Copy link
Contributor

@pietrop pietrop commented Aug 12, 2019

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

#160

Describe what the PR does

see
#160 (comment)
#160 (comment)

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

Ready for review (cc @jamesdools )

Additional context

NA

@pietrop pietrop changed the title Develop should component update refactor Develop branch - should component update refactor Aug 12, 2019
@pietrop
Copy link
Contributor Author

pietrop commented Aug 12, 2019

hesitant to do MediaPlayer coz the props and state attributes are compared against each other, because of the nature of the inner working of the component.

  shouldComponentUpdate(nextProps, nextState) {
    if (nextProps.rollBackValueInSeconds !== this.state.rollBackValueInSeconds) {
      return true;
    }
    if (nextProps.timecodeOffset !== this.state.timecodeOffset) {
      return true;
    }
    // TODO: workaround to keep the hook functions, only call re-renders
    // if current time has changed. And it seems eliminate component's unecessary re-renders.
    if (nextProps.currentTime !== this.props.currentTime) {
      return true;
    }

    if (nextState.playbackRate !== this.state.playbackRate) {
      return true;
    }

    if (nextProps.mediaDuration !== this.props.mediaDuration ) {
      return true;
    }

    if (nextState.isMute !== this.state.isMute) {
      return true;
    }

    return false;
  }

@pietrop pietrop merged commit fe495f2 into develop Aug 12, 2019
pietrop pushed a commit that referenced this pull request Aug 13, 2019
* added support for docx - needs better integration altho it works

* adding timecodes and speakers to plain text export

* develop: Fix 159 performance problem (#171)

* moved header into a component

with shouldComponentUopdate false to avoid unecessary re-renders - test

* moved Header component into separate file

+removed unused styling, commented out for now in case it's needed, eg in mobile view?

* moved playback_rates const into separate file

* optimised re-render of playback rate

* optimise re-render for VideoPlayer

* added some notes - draft

on how to prevent uncessary re-renders in React

* Added some comments

* small note in docs

about using console.log in render to measure performance

* ammend to notes

* Update 2019-05-16-prevent-unnecessary-re-renders-in-react.md

* notes update

* notes fix

* trying out why-did-you-update

* updated MediaPlayer and subcomponents

* made ToolTip 'how does this work' into it's own component

* updated Demo app to reduce unecessary re-renders

* added react-visibility-sensor

* refactor settings

* removed unecessary attributes from state of components + WrapperBlock performance tweak using react-visibility-sensor

* develop: Update timestamps diff (#172)

* Added timestamp update via diff tool

* Added missing function

* Commited intermediate state

* Rewrote timestamp alignment and differ to be integrated in each other instead of doing a 2-step process.

* Update Timestamps now works correctly.

* Fixed errors from rebase, removed debug code

* Moved UpdateTimestamp into its own folder.

* added updateTimestampsSSTAlign which updates the timestamps with the sst-align code

* Added documentation

* Merged timer for updating the timestamps and local save.

* Selection state is now kept across updates to timestamps

* Fixed bug where words with punctuation always are considered as new words. Timestamp update function now also uses the alignWords function directly instead of alignJSONText, removing some overhead.

* Fixed small bug which raised an error if an empty block was present during timestamp update

* Changed time of timestamp-update. Now re-calculates the timestamps after 5 seconds if the transcript has been edited or if the user saves the transcript manually with the save button

* Code cleanup

* develop: Murezzda update timestamps diff (#173)

* Added timestamp update via diff tool

* Added missing function

* Commited intermediate state

* Rewrote timestamp alignment and differ to be integrated in each other instead of doing a 2-step process.

* Update Timestamps now works correctly.

* Fixed errors from rebase, removed debug code

* Moved UpdateTimestamp into its own folder.

* added updateTimestampsSSTAlign which updates the timestamps with the sst-align code

* Added timestamp update via diff tool

* Added missing function

* Commited intermediate state

* Rewrote timestamp alignment and differ to be integrated in each other instead of doing a 2-step process.

* Update Timestamps now works correctly.

* Fixed errors from rebase, removed debug code

* Moved UpdateTimestamp into its own folder.

* added updateTimestampsSSTAlign which updates the timestamps with the sst-align code

* Added documentation

* Merged timer for updating the timestamps and local save.

* Selection state is now kept across updates to timestamps

* Fixed bug where words with punctuation always are considered as new words. Timestamp update function now also uses the alignWords function directly instead of alignJSONText, removing some overhead.

* Fixed small bug which raised an error if an empty block was present during timestamp update

* Changed time of timestamp-update. Now re-calculates the timestamps after 5 seconds if the transcript has been edited or if the user saves the transcript manually with the save button

* Code cleanup

* some changes to show sudgestions for PR

* added some of changes sudgested in PR

* Update package.json

* develop: Murezzda update timestamps diff dpe groups words by speaker (#174)

* Added timestamp update via diff tool

* Added missing function

* Commited intermediate state

* Rewrote timestamp alignment and differ to be integrated in each other instead of doing a 2-step process.

* Update Timestamps now works correctly.

* Fixed errors from rebase, removed debug code

* Moved UpdateTimestamp into its own folder.

* added updateTimestampsSSTAlign which updates the timestamps with the sst-align code

* Added timestamp update via diff tool

* Added missing function

* Commited intermediate state

* Rewrote timestamp alignment and differ to be integrated in each other instead of doing a 2-step process.

* Update Timestamps now works correctly.

* Fixed errors from rebase, removed debug code

* Moved UpdateTimestamp into its own folder.

* added updateTimestampsSSTAlign which updates the timestamps with the sst-align code

* Added documentation

* Merged timer for updating the timestamps and local save.

* Selection state is now kept across updates to timestamps

* Fixed bug where words with punctuation always are considered as new words. Timestamp update function now also uses the alignWords function directly instead of alignJSONText, removing some overhead.

* Fixed small bug which raised an error if an empty block was present during timestamp update

* Changed time of timestamp-update. Now re-calculates the timestamps after 5 seconds if the transcript has been edited or if the user saves the transcript manually with the save button

* Code cleanup

* some changes to show sudgestions for PR

* added some of changes sudgested in PR

* adjusted DPE adapter

so that it preserves paragraphs break within contiguos speakers

* fixed one test

* commented out auto align

left aligning as a step before save btn and before export function, rather then as a step that happens everytime autosave is triggered, as that might be unecessary, and add performance overhead, I also noticed the cursor position jumped after realignement, thought something was been put in place to preserve/avoid that?

* updated package-lock

* fixed vulneranilities

* Getting ready to publish alpha

* 1.0.4-alpha.0

* 1.0.4@alpha

* fixed notes

* changing speaker and timecodes to be unselectable

* 1.0.4-alpha.1

* 1.0.4-alpha.1

unselectabel speakers and timecodes

* fix speaker and timecodes at paragraph level after realignement

* 1.0.4-alpha.2

* fixed docx integration

* Subtitles export (#168)

* added option to export srt files

and layout to export other type of captions with auto segmentation of lines

* added support for other subtitles formats

* fixed npm audit

* implemented export in UI

* fixed test

added sample files for adding tests for subtitle composer module at later stage

* added optional analytics

for export download options

* updated CSS

* 1.0.4-alpha.3

* fixed subtiles segmentation

algo was picking the wrong words from the list

* moved PR template in github folder

* cleaned up code for subtitles parsing

* 1.0.4-alpha.4

* fixed alignement algo

after interpolation words time boundares  where overlapping

* fixed interpolation

* fixed filename of word doc export

* fixed TimeBox

and playback rate not working

* 1.0.4-alpha.5

* removed scroll sync (#181)

fix #180

* refactor

changes from James review #160

* Develop branch -  should component update refactor (#182)

* Refactor should component updatre for transcript editor

* Refactor should component updatre for PlayerControls

* Refactor should component updatre for TimeBox

* Refactor should component update for ProgressBar

* Refactor should component update for TimedTextEditor

* Refactor should component update for Header

* fix from PR review

Fixed from James comments from #144 (review)

* Update package.json

* Update package.json
@jamesdools jamesdools deleted the develop-should-component-update-refactor branch December 4, 2019 12:55
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.

None yet

2 participants