You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by 1ambda <gi...@git.apache.org> on 2017/03/22 20:32:21 UTC
[GitHub] zeppelin pull request #2177: [ZEPPELIN-2301] DON'T overwrite editor text whe...
GitHub user 1ambda opened a pull request:
https://github.com/apache/zeppelin/pull/2177
[ZEPPELIN-2301] DON'T overwrite editor text when note is renamed, created, ...
### What is this PR for?
DON'T overwrite local text change in editor when note is renamed, created, and so on
- `Move up paragraph` and `Move down paragraph` and other actions calling `saveNote` will reset local text change still. I will resolve it in different PRs.
#### Implementation Details
We can't get children's `$scope` in angular. That's the reason why `paragraphText` is written as a (global) service.
### What type of PR is it?
[Bug Fix]
### Todos
NONE
### What is the Jira issue?
[ZEPPELIN-2301](https://issues.apache.org/jira/browse/ZEPPELIN-2301)
Related with
- https://github.com/apache/zeppelin/pull/2120 (different approach)
- https://issues.apache.org/jira/browse/ZEPPELIN-1492 (people are complaining on this issue. but different topic.)
### How should this be tested?
1. Open the same note in 2 browsers (use different sessions e.g normal chrome + secret chrome or firefox + safari)
2. Prepare a paragraph synchronized among sessions.
3. Modify text in one browser.
4. (Create, Delete) Rename other notes. The actions which triggering `NOTE` websocket messages should not overwrite local text change in editor.
### Screenshots (if appropriate)
#### Before
#### After
### Questions:
* Does the licenses files need update? - NO
* Is there breaking changes for older versions? - NO
* Does this needs documentation? - NO
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/1ambda/zeppelin ZEPPELIN-2301/should-not-overwirte-local-text-for-note-action
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zeppelin/pull/2177.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #2177
----
commit 2d79902e2cc3e1dbc2f379dd702469e0ba5e2e63
Author: 1ambda <1a...@gmail.com>
Date: 2017-03-22T20:12:26Z
fix: DON'T overwrite local change when note action
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #2177: [ZEPPELIN-2301] DON'T overwrite editor text when note ...
Posted by 1ambda <gi...@git.apache.org>.
Github user 1ambda commented on the issue:
https://github.com/apache/zeppelin/pull/2177
@AhyoungRyu Could you help review?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #2177: [ZEPPELIN-2301] DON'T overwrite editor text when note ...
Posted by FRosner <gi...@git.apache.org>.
Github user FRosner commented on the issue:
https://github.com/apache/zeppelin/pull/2177
Gotcha @1ambda,
The gifs make it clear now, thanks! Not sure about the exact code changes as I'm not that familiar with the code base you're touching. Do you have any specific part I should dive into or can someone else also take a look?
I agree with the priorities as well. If we combine it with save-on-lost-focus the time window is at least lower to get your changes overwritten.
Just one more related question: I noticed that there is a save timer for notebooks in the code. I tried to figure out when it is triggered but I didn't succeed. What is it used for? When is it triggered? What's the difference between saveNote and saving of a paragraph?
Thanks!
Frank
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #2177: [ZEPPELIN-2301] DON'T overwrite editor text when note ...
Posted by AhyoungRyu <gi...@git.apache.org>.
Github user AhyoungRyu commented on the issue:
https://github.com/apache/zeppelin/pull/2177
Sure. I'll test it and get back you soon.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin pull request #2177: [ZEPPELIN-2301] DON'T overwrite editor text whe...
Posted by 1ambda <gi...@git.apache.org>.
Github user 1ambda closed the pull request at:
https://github.com/apache/zeppelin/pull/2177
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #2177: [ZEPPELIN-2301] DON'T overwrite editor text when note ...
Posted by 1ambda <gi...@git.apache.org>.
Github user 1ambda commented on the issue:
https://github.com/apache/zeppelin/pull/2177
@FRosner I updated GIF.
What #2176 and #2177 is trying to prevent local change from overwritten. Overwriting occurs in many cases.
- #2176 is only about **when other browsers run the same paragraph**
- #2177 is related with **the case when people rename, remove, create notes**
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin pull request #2177: [ZEPPELIN-2301] DON'T overwrite editor text whe...
Posted by 1ambda <gi...@git.apache.org>.
GitHub user 1ambda reopened a pull request:
https://github.com/apache/zeppelin/pull/2177
[ZEPPELIN-2301] DON'T overwrite editor text when note is renamed, created, ...
### What is this PR for?
DON'T overwrite local text change in editor when note is renamed, created, and so on
- `Move up paragraph` and `Move down paragraph` and other actions calling `saveNote` will reset local text change still. I will resolve it in different PRs.
#### Implementation Details
We can't get children's `$scope` in angular. That's the reason why `paragraphText` is written as a (global) service.
### What type of PR is it?
[Bug Fix]
### Todos
NONE
### What is the Jira issue?
[ZEPPELIN-2301](https://issues.apache.org/jira/browse/ZEPPELIN-2301)
Related with
- https://github.com/apache/zeppelin/pull/2120 (different approach)
- https://issues.apache.org/jira/browse/ZEPPELIN-1492 (people are complaining on this issue. but different topic.)
### How should this be tested?
1. Open the same note in 2 browsers (use different sessions e.g normal chrome + secret chrome or firefox + safari)
2. Prepare a paragraph synchronized among sessions.
3. Modify text in one browser.
4. (Create, Delete) Rename other notes. The actions which triggering `NOTE` websocket messages should not overwrite local text change in editor.
### Screenshots (if appropriate)
#### Before
#### After
### Questions:
* Does the licenses files need update? - NO
* Is there breaking changes for older versions? - NO
* Does this needs documentation? - NO
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/1ambda/zeppelin ZEPPELIN-2301/should-not-overwirte-local-text-for-note-action
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zeppelin/pull/2177.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #2177
----
commit 2d79902e2cc3e1dbc2f379dd702469e0ba5e2e63
Author: 1ambda <1a...@gmail.com>
Date: 2017-03-22T20:12:26Z
fix: DON'T overwrite local change when note action
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #2177: [ZEPPELIN-2301] DON'T overwrite editor text when note ...
Posted by FRosner <gi...@git.apache.org>.
Github user FRosner commented on the issue:
https://github.com/apache/zeppelin/pull/2177
I am not sure I understand the exact difference between this and #2176. Can you try to upload bigger versions of the gif or elaborate a bit on the difference? Are the two PR (#2176 and #2177) fixes for the same problem but in a different way or are they addressing different problems?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #2177: [ZEPPELIN-2301] DON'T overwrite editor text when note ...
Posted by 1ambda <gi...@git.apache.org>.
Github user 1ambda commented on the issue:
https://github.com/apache/zeppelin/pull/2177
Let me implement locking and realtime notification soon. I will close this PR.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin pull request #2177: [ZEPPELIN-2301] DON'T overwrite editor text whe...
Posted by 1ambda <gi...@git.apache.org>.
Github user 1ambda closed the pull request at:
https://github.com/apache/zeppelin/pull/2177
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #2177: [ZEPPELIN-2301] DON'T overwrite editor text when note ...
Posted by 1ambda <gi...@git.apache.org>.
Github user 1ambda commented on the issue:
https://github.com/apache/zeppelin/pull/2177
@FRosner Anything would be greatly appreciated. For example trying this PR, suggestion for how to write test code for this, and so on.
regarding to triggering auto-save, Zeppelin send `startSavveTimer` broadcast message, but there is no `$scope.$on('startSaveTimer', ...)`. That's the reason why auto save is not working properly.
```
app/notebook/paragraph/paragraph.controller.js
602: $scope.$broadcast('startSaveTimer');
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #2177: [ZEPPELIN-2301] DON'T overwrite editor text when note ...
Posted by FRosner <gi...@git.apache.org>.
Github user FRosner commented on the issue:
https://github.com/apache/zeppelin/pull/2177
Thanks @1ambda. I'm quite busy at this moment so if someone else (e.g. @AhyoungRyu) can also take a look that would be greatly appreciated.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---