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

Editor performance improvements #1798

Merged
merged 67 commits into from
Dec 13, 2017
Merged

Editor performance improvements #1798

merged 67 commits into from
Dec 13, 2017

Conversation

taylortom
Copy link
Member

@taylortom taylortom commented Nov 27, 2017

Have removed much of the shared data from the editor (which was preloaded at run-time), and replaced with a more ad-hoc method, which only fetches the relevant data as it's needed.

Fixes #1758.

Main points

  • Removes global course data in Origin.editor.data:
    • contentobjects
    • articles
    • blocks
    • components
    • courseassets
    • clipboards
  • Model data is now accessed in a more ad-hoc way when it's needed, rather than pre-loading everything at once
  • ContentModels no longer do a compulsory fetch in their initialize function, which was causing lots of unnecessary requests
  • Course data models have been reworked to work asynchronously, pay particular attention to: getParent/getChildren/getSiblings which are now fetchParent/fetchChildren/fetchSiblings respectively (ContentModel.js)
  • Have slightly reworked any files I've touched where I thought it was needed:
    • Fixed major whitespace issues (incorrect indentation/extra line-breaks etc.)
    • Some files were very confusing, so have moved some functions around to make everything more readable
    • Have removed unused/redundant code where I spotted it

Known issues

  • I don't like the way the page animates gradually/blocks pop into view, so going to look into rendering something (perhaps setting a min-height on the block) to show something's happening without the pop
  • Louise's preview popup blocker fix is broken due to async course validation
  • Pasting within a page causes a re-render of the entire page
  • Issue with menu editor not rendering layers correctly. For example:
    • Add a page/sub-menu to a sub-menu
    • Click on the new item
    • Sub-menu items disappear
    • Refreshing the page shows all missing items as expected
  • Getting a TypeError in EditorPageComponentView (when getSupportedLayout is called by evaluateLayout)
  • CKEditor errors when loading project settings for the second time (causing a broken body input)
  • Need to replace all notify calls using 'xxxxx'

taylortom and others added 8 commits November 28, 2017 11:15
* Make content fetches run in parallel

* Make helper parallel

* Improve page editor rendering time

* Remove mandatory fetch from contentModel.initialize

* Make sure model is fetched before rendering

* Remove unused import

* Move async rendering to EditorPageBlockView

* Remove client clipboard data

Clipboard data now automatically deleted by back-end

* Fix issue with hidden blocks
@taylortom taylortom changed the base branch from develop to release/v0.4.1 December 4, 2017 12:30
taylortom and others added 6 commits December 6, 2017 14:02
* Make content fetches run in parallel

* Make helper parallel

* Improve page editor rendering time

* Remove mandatory fetch from contentModel.initialize

* Make sure model is fetched before rendering

* Remove unused import

* Move async rendering to EditorPageBlockView

* Remove client clipboard data

Clipboard data now automatically deleted by back-end

* Fix issue with hidden blocks

* Add mid-render style to editor page blocks

* Fix preview with popup blockers

* Stop page re-rendering on paste

* Remove logs

* Handle destroy async

* Fix menu layer rendering for new/removed COs

* Make rendering series
@taylortom
Copy link
Member Author

taylortom commented Dec 7, 2017

All known issues fixed, please retest when you have time 😊

Copy link
Contributor

@tomgreenfield tomgreenfield left a comment

Choose a reason for hiding this comment

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

Great stuff; I haven't encountered any issues while casually moving about the tool.

Are we doing anything about the xxxxx error texts in this commit?

@taylortom taylortom requested review from lc-thomasberger and removed request for simondate December 11, 2017 10:10
@canstudios-louisem
Copy link
Contributor

I've had a test on a copy of our big server and it seems a lot better navigating around can still feel a little "slow" but I'm talking 0.5-1 second and I'm happy enough for it to go into an RC.

@taylortom
Copy link
Member Author

Awesome, thanks @canstudios-louisem.

I'd be interested to have a closer look at that and try and figure out where the remaining bottlenecks (not necessary for this release though if we're happy with the improvement...).

@tomgreenfield tomgreenfield merged commit b7d8f78 into release/v0.4.1 Dec 13, 2017
@tomgreenfield tomgreenfield deleted the issue/1758 branch December 13, 2017 14:21
taylortom added a commit that referenced this pull request Dec 15, 2017
* release/v0.4.1: (22 commits)
  Editor performance improvements (#1798)
  use debounce instead of throttle
  throttle callbacks use on to properly remove scroll listener in remove function
  Fix bug with restoring dashboard preferences
  Fix issues with window resizing and dashboard paging
  Make dashboard layouts a bit less hard-coded
  Fix issues highlighted by review
  article view now generated after article model saved
  Amend check to allow undefined data
  Switch to local log func
  Stop throwing error if repo update check failed
  Fix version check
  Rewrite projectsView.js to fix pagination
  Add override for removal of list items
  Handle response errors better
  Fix bug with recursive function call
  Remove testing header
  Update CHANGELOG for release 0.4.0 (#1739)
  Enchancements to install/update (#1726)
  Amend contentModel._children to allow for arrays
  ...

# Conflicts:
#	frontend/src/modules/scaffold/views/scaffoldAssetView.js
Annoraaq pushed a commit to Annoraaq/adapt_authoring that referenced this pull request Feb 21, 2018
* Rename model attributes

* Remove unnecessary collections from editor data

* Update attribute references

* Remove unused functions

* Update models to use actual collection item names

* Refactor editor menu code to allow for async loading

* Stop re-rendering if item already selected

* Remove references to unused model attributes

* Improve comments

* Stop code executing for each item

* Refactor get functions

* Remove unused bits

* Fix various issues

* Remove unused imports

* Refactor to only call listenTo once

* Update ContentModel to return children as array

Due to issue with Backbone.Collections and multiple model types

* Remove unused cut functionality

* Remove unused code

* Update component render to work with async code

* Handle re-rendering in parent

* Make re-render async

* Fix model accessor

* Remove unused imports

* Simplify component delete code

* Stop fetchSiblings from returning self

* Add data-id to all editorOriginViews for convenience

* Removed unused ‘ancestors’ code

* Fix page getter

* Refactor new block code

* Refactor layout code

* Make destroy async to ensure we catch any errors

* Refactor for brevity

* Refactor block rendering for readability

* Refactor new article code

* Remove sibling fetch

* Fix server copy/paste

* Remove unused var

* Remove unused route

* Remove unused sync classes

* Allow for proper async rendering in page views

* Fix whitespace

* Refactor for readability

* Remove logs

* Remove asset URL helpers

As they’re now async…

* Fix courseassets

* Fix async course validation

* Remove noisy warning

* Remove log

* Fix scope issue

* Fix issue with external assets

* Move stuff around

* Remove spaghetti logic from scaffoldAsset template

* Remove log

* Remove log

* Stop unnecessary 404 errors

* Fix courseassets clean-up

* Improve helper to allow for iterators which modify the original list

* Improve performance

* Make content fetches run in parallel

* Make helper parallel

* Improve page editor rendering time

* Remove mandatory fetch from contentModel.initialize

* Make sure model is fetched before rendering

* Remove unused import

* Move async rendering to EditorPageBlockView

* Remove client clipboard data

Clipboard data now automatically deleted by back-end

* Fix issue with hidden blocks

* Fix issues

* Make content fetches run in parallel

* Make helper parallel

* Improve page editor rendering time

* Remove mandatory fetch from contentModel.initialize

* Make sure model is fetched before rendering

* Remove unused import

* Move async rendering to EditorPageBlockView

* Remove client clipboard data

Clipboard data now automatically deleted by back-end

* Fix issue with hidden blocks

* Add mid-render style to editor page blocks

* Fix preview with popup blockers

* Stop page re-rendering on paste

* Remove logs

* Handle destroy async

* Fix menu layer rendering for new/removed COs

* Make rendering series

* Fix menu item state restoration

* Fix merge issues

* Don’t break for components with no supportedLayout

* Move async fetch code to index so scaffold can render correctly

Added a multi-fetch convenience method

* Refactor for readability

* Fix var reference

* Update error messages
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.

4 participants