-
Notifications
You must be signed in to change notification settings - Fork 100
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
Allow bulk task creation on paste #2273
Allow bulk task creation on paste #2273
Conversation
Signed-off-by: Stefan Forstenlechner <[email protected]>
nextcloud#1894 Signed-off-by: Stefan Forstenlechner <[email protected]>
0e8fe65
to
0b67c40
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #2273 +/- ##
==========================================
- Coverage 27.33% 26.55% -0.78%
==========================================
Files 60 62 +2
Lines 2923 3069 +146
Branches 744 681 -63
==========================================
+ Hits 799 815 +16
- Misses 2124 2254 +130 |
@t-h-e Thank you very much. That looks very nice. The code looks good so far and I love the fact that you even provide a test. There are some lint errors that you can mostly fix with |
remove unused css remove non-useful comment Signed-off-by: Stefan Forstenlechner <[email protected]>
Thanks for the comments. I configured eslint to be run by my IDE |
@t-h-e Thanks a lot! I have a few minor code style related comments, and will try to find the time to test the functionality later this week. |
stop paste event propagation the Vue way Signed-off-by: Stefan Forstenlechner <[email protected]>
I have addressed your review comments. Let me know if there is anything else 😃 |
Sorry that it takes me so long. My Nextcloud dev env was broken, and it took me a while to fix it. I will hopefully have a look at the weekend latest. |
Hi @raimund-schluessler just wanted to check in what the status is. |
@jancborchardt Could you please add @t-h-e to nextcloud/tasks? |
@t-h-e Thanks a lot for the contribution! I finally had time to the PR and it seems to work really nicely. The only problem I found is this:
It seems that the input field still has focus and then triggers a single task creation. If you could fix this, I think we could merge the PR. I think it should be possible to programmatically focus the |
Signed-off-by: Stefan Forstenlechner <[email protected]>
Actually the focus is automatically on the first button in the dialog. The issue is, at least that is what it seems to me, that the button already reacts on As the first button seems to have the focus after showing the dialog, I prevent |
Actually you are right. Focusing |
Thanks a lot, it works a lot better now. However, whether the task input field is cleared, depends on how the modal is closed. That might be a bit unexpected. After creating the tasks, the input is:
After pasting text, but before creating tasks, the input is:
I would propose that the input field is always cleared after the tasks where created, but never cleared when canceling the task creation. |
@jancborchardt Ping 🙂 |
Signed-off-by: Stefan Forstenlechner <[email protected]>
Signed-off-by: Stefan Forstenlechner <[email protected]>
fd24801
to
55438da
Compare
I forgot a |
Note that Firefox and Chrome behave differently. In Firefox it behaves now as you described. In Chrome, the pasted text is never placed in the input field. EDIT: I have tried to change |
It works well now, I would say we can merge it.
We might be able to fix this by explicitly setting the input field value to the pasted string after canceling. |
Because, as I wrote in the issue you created, and as you can see in the milestone, it is unreleased. It will be released with v0.16.0. |
@t-h-e thanks for your hard work on this. I'm so happy to see it finally released :) |
@raimund-schluessler @t-h-e sorry for missing the ping – @t-h-e you are added to the Nextcloud org and the Tasks contributors now. :) |
Solves issue #1894
See tests in tests/javascript/unit/utils/textToTask.spec.js for all possible list structures that can be pasted to create multiple tasks in one go