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

Update the revision slider so that it does not cover the bottom of long notes #2662

Merged

Conversation

sandymcfadden
Copy link
Contributor

@sandymcfadden sandymcfadden commented Feb 11, 2021

Fix

With the update to the slider position, it was sitting over top the bottom of long notes. So you couldn't see changes happening at the end no matter how far you scrolled.
This also fixes a problem with the position of the date above the slider.

This resolves #2655

Test

  1. Have a note with enough content that fills the editor.
  2. Open the history for the note.
  3. Ensure you can scroll and see all the note content.
  4. Ensure the revision date is centered above the slider handle.
  5. Ensure the date is not cut off the side of the app.

Release

  • Fix the revision slider so that the full note is visible and the position of the revision date.

@sandymcfadden sandymcfadden requested a review from a team February 11, 2021 15:25
@sandymcfadden sandymcfadden self-assigned this Feb 11, 2021
Copy link
Member

@codebykat codebykat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I think this should also be a hotfix against release/2.7.0. I'll leave the rebase to you for practice :)

Copy link
Member

@codebykat codebykat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh also when you rebase against release/2.7.0 don't forget to add a line to the release notes :) (Just setting as Changes Requested to make sure you see this)

@sandymcfadden sandymcfadden force-pushed the fix/show-full-note-with-revision-slider-visible branch from 4add524 to 3648559 Compare February 12, 2021 12:13
@sandymcfadden sandymcfadden changed the base branch from develop to release/2.7.0 February 12, 2021 12:14
@sandymcfadden
Copy link
Contributor Author

sandymcfadden commented Feb 12, 2021

@codebykat you were right about me needing the practice. It looks like I still need more and that I messed this up again.
Also, I'm not sure I need to add anything to the release notes as there is already a line about updating the revision slider? Let me know what you think.

@sandymcfadden sandymcfadden added this to the 2.7.0 ❄️ milestone Feb 12, 2021
@sandymcfadden sandymcfadden force-pushed the fix/show-full-note-with-revision-slider-visible branch from 3648559 to 0daf99f Compare February 12, 2021 17:35
@sandymcfadden
Copy link
Contributor Author

Thanks so much, @belcherj for helping me get this PR fixed up with the git lesson.

@codebykat codebykat self-requested a review February 16, 2021 23:11
@codebykat
Copy link
Member

Also, I'm not sure I need to add anything to the release notes as there is already a line about updating the revision slider? Let me know what you think.

Yeah, fair. So what I would do is edit the release notes to add a link to this PR as well. You can see prior examples where we had more than one PR linked for a single line.

Nice work on the git fix! I love that JB and I have totally different methods of resolving that issue. Git: There is always more than one correct way to shoot yourself in the foot. 😂

@sandymcfadden
Copy link
Contributor Author

Thank you @codebykat! I've updated the release notes now.

@codebykat codebykat merged commit 29c8835 into release/2.7.0 Feb 18, 2021
@codebykat codebykat deleted the fix/show-full-note-with-revision-slider-visible branch February 18, 2021 23:11
@mokagio
Copy link
Contributor

mokagio commented Feb 18, 2021

Thanks @codebykat 🙌 !

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.

New revision slider should always allow to see whole note content
3 participants