-
-
Notifications
You must be signed in to change notification settings - Fork 39
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
Customization options #283
base: main
Are you sure you want to change the base?
Conversation
I fixed the typing errors in the code I've written. Did I break something in the build process? |
Sorry I've been a bit busy with life and work.. I'll take a look this weekend and help if I can. |
Hey! I tried to check out this branch but I get a build error when running
I'm not so sure what to do to fix it.
It looks like the build error was introduced in commit: 6f76118 |
Yes I remember running a command that might have updated packages. The error has to do with something unrelated to the changes: |
This fixes some build errors related to an unwanted typescript package upgrade
@Bubobubobubobubo @tmhglnd yes, apparently a lot of packages were upgraded automatically at some point, including typescript, which causes that error. I reverted the changes on package-lock.json and re-installed the dependencies, and it's building again. Will look into that build problem later in another issue... This looks great! I'm going to make a few more edits if you don't mind @Bubobubobubobubo
|
Thank you for taking a look! Sorry about the auto-upgrade mishap. About customization, we could save a single object in localStorage to facilitate passing the configuration around in different components. I wasn't sure how localStorage was handled so I.. did nothing 😄. I can take a look and test if needed for the following commits :) |
Hey! I just briefly tried this. Very nice! 😎 Some initial notes:
|
Most of these issues are related to the fact that I customized the One Dark theme a lot, so I guess we need to apply those on top of the selected theme.
Agree, I think those are sensible shortcuts. I'll add a "Key Shortcuts" dialog soon, so this could be further customized.
Makes sense! |
Maybe not yet, it behaves a bit strange when you're zoomed in more then 100% on the page. I have to up my github skills I think, because I'm not sure what now the best way is to get my commits to be in this fork. Should I do a pull request to this fork that then eventually will be merged with the main branch? (I guess so). In the meantime you can already check the things I'm doing here: https://github.com/tmhglnd/flok/tree/add-vim-mode |
You might be able to push to this branch by adding the repo it is based on as a remote. Not a Git wizard myself 🥹 |
I would need permission for that, now I get this error: |
I made a pull request: Bubobubobubobubo#3 Some extra things I updated:
I Also noticed that the names do show up when you hover the mouse on the little circle above the cursor line. So I guess they are still there but it is indeed some css customization that got lost in the process of adding custom themes. |
Some fixes and ideas added
@tmhglnd: I have merged your changes into the branch. |
backgroundColor: "rgba(255, 0, 255, 0.5) !important", | ||
opacity: 0.5, | ||
}, | ||
".cm-ySelectionInfo": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tmhglnd I think this .cm-ySelectionInfo
is related to the "username" cursor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes!
"& .cm-scroller": { | ||
fontFamily: `Inconsolata`, | ||
paddingLeft: "2px !important", | ||
minHeight: "100vh", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and I think this minHeight
fixes the "......." outline that appears just below your cursor, it should expand the editor div height to full screen height.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also yes!
Just a word of caution from personal experience: handling keyboard shortcuts can be quite difficult if you start to take into account that there will be multiple editing modes now. Some keyboard shortcuts will simply break because of the Vim Mode. Generally speaking, I'm not a big fan of these. A good command prompt can be tremendously better, especially since you only need to remember only one keybinding to summon it. On another topic, I think that we are at a good point to start to get a bit creative with customization options. What about adding sliders for |
Created a new pull request here Bubobubobubobubo#4 With fixes for the name badges and the unwanted dotted |
Also true, I guess we need to test them more. I performed a couple or days ago and had some issues with some shortcuts, for example, at one point, I tried to comment a line with CMD+/ but mistyped and pressed CMD+. and silenced everything 😨 same thing with Ctrl+Enter in Mac, which executes all the code instead of the current block.
Yes! That'd be great. We could make use of a Settings Dialog and throw everything in there 😄 I'd tackle this in a separate PR though |
This PR is rather large and unfocused so I will keep it as a draft for now. Some help is necessary to review the code in order to tame the remaining ugly bits. I don't know much about React and my frontend skills are shallow. The PR is adding some much needed customization options to Flok:
It also refactors the keybinding based CodeMirror extension system that Flok was using. Now, options are accessed through the menu:
EDIT: well, lots of typing errors to fix.