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

Use AppSidebar #1030

Merged
merged 1 commit into from
May 2, 2021
Merged

Use AppSidebar #1030

merged 1 commit into from
May 2, 2021

Conversation

raimund-schluessler
Copy link
Member

@raimund-schluessler raimund-schluessler commented May 11, 2020

Move the right side details view to vue-components AppSidebar component. Requirement for future new features such as repeating tasks, since the sidebar runs out of space and tabs are necessary. Closes #664.

Needs the latest master of nextcloud-vue components.

Build to test the current state: tasks.tar.gz

Todo:

Needs:

Nice to have:

I will move the following to follow-up PRs as this PR is already huge:

@raimund-schluessler raimund-schluessler added design Cleanup 2. developing dependencies Pull requests that update a dependency file labels May 11, 2020
@raimund-schluessler raimund-schluessler added this to the 0.14.0 milestone May 11, 2020
@codecov
Copy link

codecov bot commented May 11, 2020

Codecov Report

Merging #1030 (42b0387) into master (a072ab1) will increase coverage by 4.29%.
The diff coverage is 36.17%.

❗ Current head 42b0387 differs from pull request most recent head 0ab8d29. Consider uploading reports for the commit 0ab8d29 to get more accurate results

@@            Coverage Diff             @@
##           master    #1030      +/-   ##
==========================================
+ Coverage   27.95%   32.24%   +4.29%     
==========================================
  Files          48       54       +6     
  Lines        2740     2791      +51     
  Branches      536      537       +1     
==========================================
+ Hits          766      900     +134     
+ Misses       1827     1752      -75     
+ Partials      147      139       -8     

@raimund-schluessler raimund-schluessler force-pushed the fix/664/sidebar branch 2 times, most recently from c6c742b to c9a72d8 Compare May 15, 2020 20:16
This was linked to issues Nov 1, 2020
@raimund-schluessler raimund-schluessler force-pushed the fix/664/sidebar branch 2 times, most recently from 7fa1389 to aca4bc8 Compare April 5, 2021 08:10
@raimund-schluessler raimund-schluessler force-pushed the fix/664/sidebar branch 6 times, most recently from b41d630 to ad68fd2 Compare April 8, 2021 21:42
@szaimen
Copy link
Contributor

szaimen commented Apr 27, 2021

and reworked the notes component to use the same logic as priority and dates. So I hope the issue of the notes not saving is resolved. Please have a look:

Great! Unfortunately, I can still reproduce this behaviour but I've found out that If I close the keyboard first and then click anywhere, it saves the changes that I've made. But this is of course a bit counter-intuitive...

@raimund-schluessler
Copy link
Member Author

and reworked the notes component to use the same logic as priority and dates. So I hope the issue of the notes not saving is resolved. Please have a look:

Great! Unfortunately, I can still reproduce this behaviour but I've found out that If I close the keyboard first and then click anywhere, it saves the changes that I've made. But this is of course a bit counter-intuitive...

Hm, very weird. I use the same approach as for priority, progress and dates. And it works well for all devices I have available. I don't know if I can debug this.

@raimund-schluessler
Copy link
Member Author

@szaimen In case you have access to the console, do you see anything regarding notes when clicking outside?

@szaimen
Copy link
Contributor

szaimen commented Apr 28, 2021

No, there is unfortunately nothing in the logs. I've made a short video demonstrating this behaviour:
DevTools - 192.168.178.27_apps_tasks_#_collections_all_tasks_6B24C7EF-66EB-4679-B0D7-21805332D14D.ics 2021-04-28 13-03-24.zip

@raimund-schluessler
Copy link
Member Author

No, there is unfortunately nothing in the logs. I've made a short video demonstrating this behaviour:
DevTools - 192.168.178.27_apps_tasks_#_collections_all_tasks_6B24C7EF-66EB-4679-B0D7-21805332D14D.ics 2021-04-28 13-03-24.zip

I can't really see where you click (or touch?) in this video. However, I will pack a test version which logs to the console when certain functions are called. Maybe we can figure out what is not called then.

@raimund-schluessler
Copy link
Member Author

@szaimen And could you please test, if the note is saved when you click somewhere into the AppSidebar, e.g. onto the Notes tab?

@szaimen
Copy link
Contributor

szaimen commented Apr 28, 2021

And could you please test, if the note is saved when you click somewhere into the AppSidebar, e.g. onto the Notes tab?

I've already done that. It's the same: the change only gets saved if I close the keyboard first.

@raimund-schluessler
Copy link
Member Author

@szaimen I build a version which prints to the console when the sidebar is closed, when the notes, priority and dates are saved and when the components are unmounted or destroyed (which should lead to saving the edited value). Please have a look and let me know what the console shows when you edit a note.

tasks.tar.gz

If this also doesn't help to figure out the problem: Is the simulator you showed in the video freely available? Then I could just test it like you.

@szaimen
Copy link
Contributor

szaimen commented Apr 29, 2021

I've figured out what the problem is. It seems to be related to the keyboard that seems to block any input due to the word suggestions. When I validate the word that I try to type or type in a space or backspace, everything in the notes field gets safed and hence it works.
But as long as it is only one word that isn't validated, yet, this error always occurs. Since this seems to be related to the way how inputs seem to work here (Android with chromium based browser and android keyboard), I guess this is no error that you can fix in the tasks app.
@raimund-schluessler Sry for this and thank you for your efforts!

@szaimen
Copy link
Contributor

szaimen commented Apr 29, 2021

BTW: what you call a simulator is actually the official way how to debug Android Chromium sessions ;) So you would still need an Android phone to do this...

@raimund-schluessler
Copy link
Member Author

raimund-schluessler commented Apr 29, 2021

I've figured out what the problem is. It seems to be related to the keyboard that seems to block any input due to the word suggestions. When I validate the word that I try to type or type in a space or backspace, everything in the notes field gets safed and hence it works.
But as long as it is only one word that isn't validated, yet, this error always occurs. Since this seems to be related to the way how inputs seem to work here (Android with chromium based browser and android keyboard), I guess this is no error that you can fix in the tasks app.
@raimund-schluessler Sry for this and thank you for your efforts!

Ok, thanks for having a look. Seems a bit unfortunate, but really sounds as if fixing in the Tasks app is difficult.
@szaimen I guess it doesn't work for the current version of the app either?

@raimund-schluessler
Copy link
Member Author

BTW: what you call a simulator is actually the official way how to debug Android Chromium sessions ;) So you would still need an Android phone to do this...

Ah, alright. This wouldn't help much in my case 🙈

@szaimen
Copy link
Contributor

szaimen commented Apr 29, 2021

@szaimen I guess it doesn't work for the current version of the app either?

Actually, somehow it seems to work there. So it always safes correctly with the current version...

@raimund-schluessler
Copy link
Member Author

@szaimen Here comes my last try to fix this for now. I added back a @change handler which I removed during refactoring, but which is there in the current release. Maybe this makes a difference.

tasks.tar.gz

@szaimen
Copy link
Contributor

szaimen commented Apr 29, 2021

Thanks! Unfortunately, it still doesn't work but since this error only occurs if you type in only one word, I guess it is fine 👍

@raimund-schluessler
Copy link
Member Author

I will merge this now, since I want to go on with moving to material design icons and cleaning up the SCSS files. Feedback is still very welcome, we will handle it in follow-up PRs.

Signed-off-by: Raimund Schlüßler <[email protected]>
@raimund-schluessler raimund-schluessler merged commit d533405 into master May 2, 2021
@delete-merged-branch delete-merged-branch bot deleted the fix/664/sidebar branch May 2, 2021 07:20
@szaimen
Copy link
Contributor

szaimen commented May 2, 2021

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cleanup dependencies Pull requests that update a dependency file design
Projects
None yet
2 participants