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

added fastscroll functionality and some layout changes inside the note view #643

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

HM201
Copy link

@HM201 HM201 commented Dec 22, 2018

I added a fastscrollscrollview in the notes layout upon request on issue #523
I made some layout changes in the notes layout based on user request on issue #434

…lso added some changes in note layout upon user request
@federicoiosue
Copy link
Owner

Hi Misham,

thanks for your contribution. However there are some problems to be solved before merging your pull request:

  1. Your code misses both UI and unit testing.

  2. The first reason of missing tests is that the code has been took from this library so I'm asking: why copying code (that would also increase technical debt of Omni Notes) instead of using that as external library? Yes that library does not contains tests neither but eventual changes or fixes made to it would have been easier to include into ON later.

  3. The fastscroll code/layout modification causes a problem with the position of the scrollbar itself, as shown into this screenshot
    12

  4. The note's detail space layout modification causes the following whenever a note is going to be modified, except than when is is long enough to be scrolled vertically

android_emulator_-_nexus_5x_api_28_x86_5554

Great work for now, I've tried and this change really seems useful!

@federicoiosue federicoiosue reopened this Dec 26, 2018
@federicoiosue federicoiosue force-pushed the develop branch 2 times, most recently from fd07c2c to 0bb927a Compare January 17, 2020 00:42
@federicoiosue federicoiosue force-pushed the develop branch 6 times, most recently from 9d11b98 to 0f78b8c Compare July 31, 2020 08:43
@federicoiosue federicoiosue force-pushed the develop branch 3 times, most recently from 73fc7a0 to c87d5ee Compare August 5, 2020 09:58
@federicoiosue federicoiosue force-pushed the develop branch 4 times, most recently from cfb925a to db0286b Compare August 14, 2020 22:04
@federicoiosue federicoiosue force-pushed the develop branch 2 times, most recently from 0f1f367 to c8987f1 Compare October 2, 2020 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants