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 added this to To do in Omni Notes Jan 16, 2020
@federicoiosue federicoiosue force-pushed the develop branch 6 times, most recently from 9d11b98 to 0f78b8c Compare July 31, 2020 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Omni Notes
  
To do
Development

Successfully merging this pull request may close these issues.

None yet

2 participants