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

Desktop: Add monospace enforcement for certain elements #4689

Merged
merged 5 commits into from
Mar 26, 2021

Conversation

CalebJohn
Copy link
Collaborator

This adds forced monospace for Tables, Code blocks and Checkboxes in the CodeMirror editor.

This was briefly discussed on the forum

For checkboxes and code blocks, I piggy back on the CodeMirror highlighting and add the monospace class.

For tables I couldn't do that. Instead table check for a valid header, and then enter table mode. This just means that all lines following the valid header will be monospace until a line is reached that does not contain a '|' character.

I also enforce monospace on any line that begins with a '|', the assumption being that those lines are likely going to become a part of a table, and it's better to start writing in monospace rather than snapping to monospace once the header is finished.

Something I haven't done yet is to add a setting that enables this behaviour. I'm not sure if it will be necessary.
We'll at least need a way for the user to specify which monospace font they want to use.

Checkboxes, Code blocks, and tables will all have
a monospace font enforced
This allows the use of proportional fonts
@CalebJohn CalebJohn marked this pull request as draft March 16, 2021 21:11
@tessus
Copy link
Collaborator

tessus commented Mar 16, 2021

I'm already using a monospace font in Joplin (Codemirror editor). Does this override my settings? If so, can I please have an option to disable this behavior?

P.S.: What I meant was, will my font change to a different monospace font?

But I think that your last sentence answers my question, unless it will just use the one I'm already using:

We'll at least need a way for the user to specify which monospace font they want to use.

I can test it later tonight.

@CalebJohn
Copy link
Collaborator Author

@tessus

Yes it will override your set font. I've just pushed another commit that adds an option for for the monospace font, so you'll need to set both if you want them to be the same.

@tessus
Copy link
Collaborator

tessus commented Mar 19, 2021

Btw, it's still in a draft state.

@tessus tessus added desktop All desktop platforms editor labels Mar 19, 2021
@CalebJohn CalebJohn marked this pull request as ready for review March 21, 2021 00:18
@CalebJohn
Copy link
Collaborator Author

Thanks @tessus Just wanted to give it another review myself before taking it out of the draft state.

As a summary, this PR enforces monospace for

  • Tables
  • Checkboxes
  • Code (blocks, indented, inline)

@laurent22
Copy link
Owner

Thanks for the pull request @CalebJohn, that looks good. What would be the default fonts? Is it monospace everywhere by default or regular font for text, and monospace for table, code, etc.?

@CalebJohn
Copy link
Collaborator Author

What would be the default fonts? Is it monospace everywhere by default or regular font for text, and monospace for table, code, etc.?

It's monospace everywhere by default. Currently, there are still cases where the transition from proportional font -> monospace, can cause a "pop" when the width of text changes, so for users sake I left monospace as the default.
I can make "Open Sans" the default in settings though. Just give me the word.

@tessus
Copy link
Collaborator

tessus commented Mar 23, 2021

I've just tested it and it seems that if the second one is left empty, it will still use my monospace font, and not a random one.

so you'll need to set both if you want them to be the same.

Apparently that's not the case for some reason. Pretty cool.

image

@laurent22
Copy link
Owner

It's monospace everywhere by default. Currently, there are still cases where the transition from proportional font -> monospace, can cause a "pop" when the width of text changes, so for users sake I left monospace as the default. I can make "Open Sans" the default in settings though. Just give me the word.

Yes if you could make the proportional font the default that would be great, as I think it's the better default. Let's see what feedback we get and if it causes problem we can review the default afterwards.

I'm not sure Open Sans is available everywhere? Would it make sense to make the generic "sans-serif" as default instead?

Open sans is the new editor font default
sans-serif is the new editor font fallback
editor font fallback is now Avenir, Arial, sans-serif
@CalebJohn
Copy link
Collaborator Author

Apparently that's not the case for some reason. Pretty cool.

Sadly, that's a bug. I've just fixed it in this commit.

I'm not sure Open Sans is available everywhere? Would it make sense to make the generic "sans-serif" as default instead?

For some reason I thought Open Sans was the default viewer font. I changed the fallback font to use Avenir, Arial, sans-serif, to align with the viewer. I undid the previous change that adds Open Sans.

@laurent22
Copy link
Owner

All good then, let's merge.

@laurent22 laurent22 merged commit 81b3ddf into laurent22:dev Mar 26, 2021
@CalebJohn CalebJohn deleted the editor-mono branch March 26, 2021 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
desktop All desktop platforms editor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants