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

refactor: refactored the positioning system of the toolbar in the "following". #150

Merged
merged 9 commits into from
Apr 10, 2024

Conversation

s4my
Copy link

@s4my s4my commented Apr 6, 2024

  • Refactor the way the toolbar's position is calculated for the "following" mode to make sure the toolbar:

    • Is always visible, even when we the user selects the last or first line in the editor. (which was not the case before this commit)
    • To try not to overflow over the File Explorer or the Outline.
  • Refactord isExistoolbar() to get rid of deprecated code (i.e., activeLeaf), and to simplify the code.

  • Refactored getModestate() into isSource(): since getModestate() doesn't really get the mode state, but rather returns whether the MarkdownView mode state is "source" or not, I renamed it to isSource() to reflect just that, and simplified the function implementation in the process.

s4my added 2 commits April 6, 2024 10:40
"following".

- Refactord isExistoolbar() to get rid of deprecated code
  (i.e., `activeLeaf`), and to simplify the code.

- Refactor the way the toolbar's position is calculated for the
  "following" mode, to make sure the toolbar:
  - Is always visible, even when we the user selects the last
    line in the editor.
  - Never overflows over the File Explorer of the Outline.
"following".

- Refactord isExistoolbar() to get rid of deprecated code
  (i.e., `activeLeaf`), and to simplify the code.

- Refactor the way the toolbar's position is calculated for the
  "following" mode, to make sure the toolbar:
  - Is always visible, even when we the user selects the last
    line in the editor.
  - Never overflows over the File Explorer of the Outline.
@cumany
Copy link
Collaborator

cumany commented Apr 7, 2024

Thank you for your work. There seems to be a positioning issue with my computer; the toolbar keeps overflowing off the screen. We'll hold off on merging branches until it's resolved. More improvements to come.

@s4my
Copy link
Author

s4my commented Apr 7, 2024

the toolbar keeps overflowing off the screen.

Can you elaborate more please, what do you mean off screen? like outside of Obsidian's window completely? and can you please make sure to disable any plugins or themes that might interfere with your testing (you could use Obsidian sandbox vault).
I updated the code, please try again and keep me updated.

s4my added 2 commits April 7, 2024 05:14
"following".

- Refactord isExistoolbar() to get rid of deprecated code
  (i.e., `activeLeaf`), and to simplify the code.

- Refactor the way the toolbar's position is calculated for the
  "following" mode, to make sure the toolbar:
  - Is always visible, even when we the user selects the last
    line in the editor.
  - Never overflows over the File Explorer or the Outline.
Since getModestate() doesn't really get the mode state but rather
returns whether the MarkdownView mode state is "source" or not, I
renamed it to isSource(), and simplified the function in the process.
@cumany
Copy link
Collaborator

cumany commented Apr 7, 2024

it looks a bit more normal, but the following list of tests will help you to determine if you have the same problem:

  1. Multiple rows are selected from left to right and the toolbar is obscuring the center.
    image
  2. selecting words at the end of a line from left to right completely obscures the words.
    image
  3. This problem seems to be a little more pressing, as in workplace panel with a left/right split screen, when you select text on the right side of the split screen, the toolbar is still in the same place as on the left side of the split screen.
    image

@s4my s4my force-pushed the refactor/position branch 4 times, most recently from 82e1f53 to 09e6246 Compare April 8, 2024 08:12
@s4my
Copy link
Author

s4my commented Apr 8, 2024

Hi @cumany, thank you for the detailed feedback, I fixed all the issues you mentioned above in this new commit. If you need me to make any further changes feel free to tell.

@s4my s4my force-pushed the refactor/position branch 4 times, most recently from b38e265 to e0df83d Compare April 9, 2024 02:37
position is set to "following".

- Refactored isExistoolbar() to get rid of deprecated code
  (i.e., `activeLeaf`), and to simplify the code.

- Refactor the way the toolbar's position is calculated for the
  "following" mode, to make sure the toolbar:
  - Is always visible, even when we the user selects the last
    or first line in the editor.
  - To try not to overflow over the File Explorer or the Outline.

- Refactored getModestate() into isSource().
  Since getModestate() doesn't really get the mode state but rather
  returns whether the MarkdownView mode state is "source" or not, I
  renamed it to isSource(), and simplified the function in the process.
@cumany
Copy link
Collaborator

cumany commented Apr 10, 2024

Good job. It was perfect!

@cumany cumany merged commit 033d24c into PKM-er:master Apr 10, 2024
@s4my s4my deleted the refactor/position branch April 10, 2024 03:13
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