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

Fixed lock button behavior and display #2661

Merged
merged 7 commits into from
Jan 23, 2019

Conversation

miguelalexbt
Copy link
Contributor

Description

Fixed the bug reported in #2121. Now, the locking state stays consistent when you change between split view and editor/preview view.
This created a new bug: after locking, changing to split view and changing back to editor/preview view, there would be no unlock icon, leaving the user "stuck" in the editor view. In order to fix that, I changed the line 22 of MarkdownEditor.js.
Finally, fixed a display bug where the lock button was still showing up in the split view.

simplescreenrecorder-2018-11-29_17 26 21

Issue fixed

#2121

Type of changes

  • πŸ”˜ : Bug fix (Change that fixed an issue)
  • βšͺ :Breaking change (Change that can cause existing functionality to change)
  • βšͺ : Improvement (Change that improves the code. Maybe performance or development improvement)
  • βšͺ Feature (Change that adds new functionality)
  • βšͺ Documentation change (Change that modifies documentation. Maybe typo fixes)

Checklist:

  • πŸ”˜ My code follows the project code style
  • βšͺ I have written test for my code and it has been tested
  • πŸ”˜ All existing tests have been passed
  • βšͺ I have attached a screenshot/video to visualize my change if possible

@ZeroX-DG ZeroX-DG added the awaiting review ❇️ Pull request is awaiting a review. label Nov 30, 2018
@ZeroX-DG
Copy link
Member

ZeroX-DG commented Dec 6, 2018

The lock button doesn't seem to display for me anymore.
ss

@miguelalexbt
Copy link
Contributor Author

That's weird. Just tested on both Linux and Windows and it works perfectly. When does it happens?

@miguelalexbt
Copy link
Contributor Author

Synced my fork with upstream and fixed what was probably causing that bug. Can you confirm if it's fixed?

@ZeroX-DG
Copy link
Member

The lock button has now appeared but it appeared in the wrong way. It appear when the editor is in split view but when I change the mode and change it back, it's gone.
boost_lock

@miguelalexbt
Copy link
Contributor Author

Found the problem: the lock button only works when using BLUR. Working on it.

@miguelalexbt
Copy link
Contributor Author

My bad, completely forgot about being able to switch between preview/editor using double/right click. The issue should be fixed now (tested on both Linux and Windows).

@ZeroX-DG
Copy link
Member

@miguelalexbt I found another bug. If you change from markdown note to a markdown note, the feature works great! But if you can it to a snippet note before changing to a markdown note, the lock button will disappear!
baasv

@ZeroX-DG ZeroX-DG added awaiting changes πŸ–ŠοΈ Pull request has been reviewed, but contributor needs to make changes. and removed awaiting review ❇️ Pull request is awaiting a review. labels Dec 19, 2018
@miguelalexbt
Copy link
Contributor Author

miguelalexbt commented Dec 19, 2018

Should be fixed now. Also added the following line, so that the editor can be blurred right after opening the markdown note (before this change, the user first had to click the editor and only then he could blur and go to the preview). If you prefer, I can revert it. https://github.com/BoostIO/Boostnote/blob/7f6d4acf90753fc452c5ac16a5b1f559a1b4e95d/browser/main/Detail/MarkdownNoteDetail.js#L69

Copy link
Member

@ZeroX-DG ZeroX-DG left a comment

Choose a reason for hiding this comment

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

Sorry for being late, LGTM πŸŽ‰ Thank you for your contribution.

@ZeroX-DG
Copy link
Member

ZeroX-DG commented Jan 6, 2019

Although I still need @Rokt33r to review this

@ZeroX-DG ZeroX-DG requested a review from Rokt33r January 6, 2019 09:08
@ZeroX-DG ZeroX-DG added approved πŸ‘ Pull request has been approved by sufficient reviewers. needs extra review πŸ”Ž Pull request requires review from an additional reviewer. and removed awaiting changes πŸ–ŠοΈ Pull request has been reviewed, but contributor needs to make changes. labels Jan 22, 2019
@Rokt33r Rokt33r merged commit 96ab8ec into BoostIO:master Jan 23, 2019
@Rokt33r Rokt33r added next release (v0.11.13) and removed needs extra review πŸ”Ž Pull request requires review from an additional reviewer. approved πŸ‘ Pull request has been approved by sufficient reviewers. labels Jan 23, 2019
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.

None yet

3 participants