Atc Picker uses Singleton instead of using topLevel #107
Conversation
EpubContainer for gh-book and WorkspaceRoot for atc
|
This definitely has problems. One of my modules in oerpub/test-book shows up multiple times "My branch new bmodule". And if I click on one of the alternates, I get Uncaught TypeError: Cannot read property 'getRoot' of undefined. I tested this on a branch that had 102 merged (listed above as a dependency). Specifically it had 92, 94, 99, 102, and 107 merged. Other stuff was unmergeable, and I didn't merge 100, which was documentation, because it had code added to it after it was reviewed. |
this causes problems when filteredCollection tries to keep its collection in sync
it should at the very least send @ as an argument
Conflicts: scripts/gh-book/app.coffee
|
I think I fixed up all the problems; duplicate entries were showing up in the ToC when you clicked on a piece of content. It was happening because the nodes in the ToC are actually Pointers to HTML files and I naively re-triggered |
|
Something weird happened when I added a new book and then went to open that book. Look at the top of the TOC pane in this screenshot. It has gone blank. |
|
I am not able to reproduce the blank ToC panel but I did reproduce the fact that the ToC panel contains all the books, not only the new "Test Book". Will look at this evening. |
|
Unfortunately no cake from testing. New books do not get new modules when you add a module. So sad. They float away. It gives this error: |
|
Fixed the add issue (had to clean up Also, merged changes made to master. |
|
Delete book is causing remote updater message and POST errors and is not saving the container.xml file. I attempted to delete the book "Delete this book" in oerpub/test-book. Deleting books on editor.oerpub.org works and doesn't generate errors. scripts/gh-book/app.coffee:173 |
|
Also adding a new module to a book triggers the save button. And if you then try to save, you get a bunch of post errors. |
|
Adding a list of things to test on this pull request. For all of these, watch the console to make sure that the right things are getting saved. The save button should not be triggered by any of the add/delete/rename actions. Add a book |
|
Is this ready for retesting? Did Tom weigh in on delete book? |
There was a problem hiding this comment.
maybe the PR title is just misleading but wasn't the point of this to not use the toplevel anymore? also the first condition of this if is empty, but i can fix that
There was a problem hiding this comment.
actually it seems like this whole section isn't needed anymore, i'm removing it unless @philschatz yells at me
There was a problem hiding this comment.
nope, remove away; somehow I missed that reference to topLevel
…sidebar instead of workspace
…tor into atc-root-singleton
There was a problem hiding this comment.
Why not just return new EpubContainer() at the end of this module? (instead of returning the EpubContainer class)
There was a problem hiding this comment.
thats what i originally did, but ran into some issue that i forget about and ended up doing it this way. could probably get it rejiggered without too much more effort but i'm not sure if its worth it
There was a problem hiding this comment.
I just mention it because other singletons in the codebase are written that way (not that I like it; it's difficult to extend a class but it's consistent). routing, app, media-types, allContent are a few examples off the top of my head
|
OK -- I went through the test suite and think we are 🍰 There is a little timing bug but probably not a regression. If you add a new module, the save all button comes on, but if you click it things go haywire. If you don't click it, it goes back off on its own when the auto-saves happen. Will make a separate pivotal ticket about that. |
Atc Picker uses Singleton instead of using topLevel
Changed the Sidebar to always use a model for rendering.
depends on #102
Testing