Change officeJs_notebook to use gadget_jsmd_eval instead of iodide_master
I changed officejs_notebook to use have the same editor as the new notebook_module and the code will be executed using gadget_jsmd_eval instead of iodide_master. the new user interface can switch between the editing area and showing the results of execution using the editable option in the left menu.
-
marked as a Work In Progress
Toggle commit list -
While I understand that the goal is to follow ERP5JS, I'm wondering if the editable checkbox is not too much for an OfficeJS app.
In ERP5JS, being in not editable allows to reduce a lot the complexity of the UI, as all empty fields are not visible. In OfficeJS, this behaviour seems too complex, as an app only has few fields.
Isn't it easier for user to get 2 links on the header to View/Edit a notebook?
Side question for @rporchetto , will it be possible in your branch to define 2 views for a portal type?
-
It's possible, it's matter of adding a new portal action with a "view" category (like object_jio_view) and set in it the path to corresponding form. Also specify in the app-router this new pair portal_type-view (like my_portal_type | my_view). Then this new view will appear grouped with the other views for this portal type in the action pages (accessible by action header button).
-
added 42 commits
-
797897fa...2a93926f - 41 commits from branch
nexedi:master - 3453adb1 - change officeJs_notebook to use gadget_jsmd_eval instead of iodide_master
Toggle commit list -
797897fa...2a93926f - 41 commits from branch
-
Toggle commit list
-
added 157 commits
-
a6a4e51e...e559ecd5 - 156 commits from branch
nexedi:master - 97a33b09 - [erp5_officejs] port notebook to gadget_jsmd_eval and add pdf conversion
Toggle commit list -
a6a4e51e...e559ecd5 - 156 commits from branch
-
Toggle commit list
-
added 43 commits
-
724e130e...55351d08 - 40 commits from branch
nexedi:master - 56bbbe1c - [erp5_property_sheets]: set default of text_content to empty string
- 4c8f7264 - [erp5_core/erp5_web_rjs_ui] jIO 3.41.0
- 06daec2e - [erp5_officejs] port notebook to gadget_jsmd_eval and add pdf conversion
Toggle commit list -
724e130e...55351d08 - 40 commits from branch
-
added 40 commits
-
fb0de9f0...314d49fb - 37 commits from branch
nexedi:master - 45856a4b - [erp5_property_sheets]: set default of text_content to empty string
- 021fb2f7 - [erp5_core/erp5_web_rjs_ui] jIO 3.41.0
- 7d154087 - [erp5_officejs] port notebook to gadget_jsmd_eval and add pdf conversion
Toggle commit list -
fb0de9f0...314d49fb - 37 commits from branch
-
added 21 commits
-
16f16b08...a953f2c6 - 18 commits from branch
nexedi:master - 834ce074 - [erp5_property_sheets]: set default of text_content to empty string
- ed59ed35 - [erp5_core/erp5_web_rjs_ui] jIO 3.41.0
- faf62c01 - [erp5_officejs] port notebook to gadget_jsmd_eval and add pdf conversion
Toggle commit list -
16f16b08...a953f2c6 - 18 commits from branch
-
Edited
Reworked so that Notebook is executed on Export Action. Then HTML attachment is stored directly from dom. This generated HTML is then used for PDF export by cloudooo. Fixed sychronisation and added missing pyodide files to appcache so python cells work offline. Basic tests passing here: https://nexedijs.erp5.net/#/test_result_module/20191002-9DD4131F
-
unmarked as a Work In Progress
Toggle commit list -
changed title from WIP: change officeJs_notebook to use gadget_jsmd_eval instead of iodide_master to Change officeJs_notebook to use gadget_jsmd_eval instead of iodide_master
Toggle commit list -
added 15 commits
-
abca8cfe...ddab98f9 - 11 commits from branch
nexedi:master - eb1befe5 - [erp5_property_sheets]: set default of text_content to empty string
- 46658360 - [erp5_core/erp5_web_rjs_ui] jIO 3.41.0
- 392079f3 - [erp5_officejs] port notebook to gadget_jsmd_eval and add pdf conversion
- c730166f - [erp5_officejs] Fix error when converting new document with remote storage
Toggle commit list -
abca8cfe...ddab98f9 - 11 commits from branch
-
added 4 commits
Toggle commit list -
added 15 commits
-
912ccc07...ddab98f9 - 11 commits from branch
nexedi:master - 7fde2b8f - [erp5_property_sheets]: set default of text_content to empty string
- 32a30a34 - [erp5_core/erp5_web_rjs_ui] jIO 3.41.0
- 3898359b - [erp5_officejs] port notebook to gadget_jsmd_eval and add pdf conversion
- 9b3ad68d - [erp5_officejs] Fix error when converting new document with remote storage
Toggle commit list -
912ccc07...ddab98f9 - 11 commits from branch
-
added 5 commits
-
2e8be7fc - 1 commit from branch
nexedi:master - 21a50462 - [erp5_property_sheets]: set default of text_content to empty string
- cf33ada0 - [erp5_core/erp5_web_rjs_ui] jIO 3.41.0
- 796890b9 - [erp5_officejs] port notebook to gadget_jsmd_eval and add pdf conversion
- 11e602e9 - [erp5_officejs] Fix error when converting new document with remote storage
Toggle commit list -
2e8be7fc - 1 commit from branch
-
added 4 commits
Toggle commit list -
added 11 commits
-
11cc831c...7a1763a9 - 8 commits from branch
nexedi:master - e99e473a - [erp5_property_sheets]: set default of text_content to empty string
- 2cb1bf92 - [erp5_officejs] port notebook to gadget_jsmd_eval and add pdf conversion
- 99c7ce23 - [erp5_officejs] Fix error when converting new document with remote storage
Toggle commit list -
11cc831c...7a1763a9 - 8 commits from branch
-
I asked @rporchetto to also check the MR, as he will be the one ensuring that no regression has been introduced in other officejs apps, and he will also update the officejs.com zip version.
Currently, I believe there are too much changes in Hamza commit and it make it hard to review.
Would it be possible to split it in multiple commits to separate the add functionalities? (use of jsmd view, update of pyodide, pdf convertion, etc).
-
-
added 7 commits
- c669ea9d - [erp5_core/erp5_web_rjs_ui] jIO 3.41.0
- 835ececa - [erp5_property_sheets]: set default of text_content to empty string
- 01ceb675 - [erp5_officejs] Fix error when converting new document with remote storage
- 7fc7196d - [erp5_officejs]: rename web documents id of notebook view to follow convention
- 0117da52 - [erp5_officejs]: notebook uses jsmd_viewer and add pdf export
- 0961f288 - [erp5_officejs]: update notebook app tests and simple export test
- 4924fb0f - [erp5_officejs]: allow to pass conversion_kw parameter to cloudooo jio storage.
Toggle commit list -
added 30 commits
-
4924fb0f...12e464cf - 24 commits from branch
nexedi:master - 263dabd9 - [erp5_property_sheets]: set default of text_content to empty string
- 3a4b7040 - [erp5_officejs] Fix error when converting new document with remote storage
- e475b4ea - [erp5_officejs]: rename web documents id of notebook view to follow convention
- 89c3df5e - [erp5_officejs]: notebook uses jsmd_viewer and add pdf export
- bc694942 - [erp5_officejs]: update notebook app tests and simple export test
- 4038e547 - [erp5_officejs]: allow to pass conversion_kw parameter to cloudooo jio storage.
Toggle commit list -
4924fb0f...12e464cf - 24 commits from branch
-
changes splitted into smaller commit and rebased to master
-
-
Could be possible to move this fix to officejs js side? for example doing some sanity checks on text_content in gadget_officejs_jio_notebook_view. Then we avoid to have this custom python code on the property and also we keep all these changes in the officejs bt only
-
The problem also exists when opening a notebook in erp5 (without officejs). If the default is not an empty string, then "None" is displayed. I think the default for text should be empty string instead of None. If we do a work around in officejs, then erp5 is not fixed.
-
@romain the change does not have any impact on existing erp5 tests. Here is the latest test result of this branch: https://nexedijs.erp5.net/#/test_result_module/20191014-8B1C7B1E
-
-
26 26 } 27 27 }) 28 28 .push(function () { 29 return gadget.repair(); 29 if (!gadget.state.cloudooo_only) { -
I confirm this bug in only office. Could it be possible to avoid an ad-hoc if in sync gadget for this case? I don't know the cloudoo convert in deep detail, but may be there's a way to change the options.redirect content in cloudoo gadget to avoid this redirect issue?
-
The problem is that the document id where to redirect to does not exist anymore after sync because the new id is generated by erp5. So officejs cannot know this new id before sycing. In theory it would be possible to redirect to document list instead, but this would be very unfriendly for the user. The user expects to come back to the document he was editing or viewing after export. Also I think the change matches more what the user wants to do: only export when clicking on export, not export and sync with erp5.
-
After digging a bit this code, it seems the
redirectparameter (naming is too generic and prevent understanding what it is...) is created AND handled inojs_gadget_erp5_page_ojs_download_convert_js.In such case, why not only modifying this gadget (which can check a
404and use another property to find the document with allDocs)?In ideal, I want to get rid of the 2 jIO storages (officejs/cloudooo) which are there as a workaround only.
-
It would also be nice to reproduce the issue in a test
-
allDocs query would be possible by using a unique property (or combination), such as modification date or reference+version+revision. Here some thoughts on possible drawbacks:
- The download_conver gadget is used in several apps, for example onlyoffice. So should we make the existance of such property a requirment for all (future) apps using this gadget ?
- As a user I want to convert without doing a full syn, this will not be possible anymore
-
I agree that the implementation could be improved with better api, but the idea of partial sync is valid in my opinion. Not only for fixing this bug but also for the following use cases:
- With flacky connection (for example train), I want only export, not waiting for the full sync
- I want export to work even when erp5 is down
- I might later want a delete button so that I can create many notebooks by cloning existing notebooks in erp5 but many are only for playing, I do not want to upload them to erp5, so I delete them before syncing. But still I want to be able to export them
-
-
-
@klaus this commit seems to include 3 big steps:
- move to jsmd viewer
- migrate local data from the
text_contentproperty to thedataproperty - activate upload and export
It is possible to split in 3 commits?
@roque do you have a proposal to start organizing such data model modification?
-
@romain it is very hard to split, because I do not have a working version of the code which uses jsmd viewer but does not use data property. It would mean that I would an effort to reprogram to use jsmd viewer but without text content. Likewise, upload and export functionality existed before the change from text_content to data: When I split the commits, then the commit for text_conent -> data would have broken upload and export.
-
-
Please register or sign in to post a comment