-
Notifications
You must be signed in to change notification settings - Fork 25
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
added keymap selection #7
base: master
Are you sure you want to change the base?
Conversation
src/app/pad.component.html
Outdated
@@ -136,6 +136,19 @@ | |||
<a class="invert" href="https://github.com/blindpad/blindpad/issues/new">feedback</a> | |||
<a class="invert" href="https://www.paypal.com/cgi-bin/webscr?cmd=_s-xclick&hosted_button_id=VW2QSSP76L36N">donate</a> | |||
<a class="invert" routerLink="/about">about</a> | |||
|
|||
|
|||
<button class="keymap-button" *ngIf="!!getPadKeymap()" (click)="onKeymapButtonClick()"> |
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.
@rmnoon Any idea why it is not displaying? getPadKeymap
evaluates to null for some reason even if I set the default value to 'vim'
Hi. Awesome software, very cleanly written. I followed your code style and got the keymap selection feature working. From now on, blindpad will be one of my main tools. |
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.
this is really nice! left a few comments. thanks for submitting!
@@ -38,6 +44,14 @@ export interface EditorMode { | |||
children?: EditorMode[]; | |||
} | |||
|
|||
export const DEFAULT_KEYMAP: string = 'vim'; |
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.
I haven't used CodeMirror keymaps but shouldn't the default be to not have one?
@@ -38,6 +44,14 @@ export interface EditorMode { | |||
children?: EditorMode[]; | |||
} | |||
|
|||
export const DEFAULT_KEYMAP: string = 'vim'; | |||
|
|||
export const KEYMAPS: string[] = [ |
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.
if you know what the expected keymaps are you can do something in typescript like export type Keymap = 'vim' | 'emacs' | 'sublime'
and then just use that type everywhere instead of string
@@ -38,6 +39,7 @@ export class AutoEditor { | |||
|
|||
getEdits(): Subject<PadEdit> { return this.edits; } | |||
getMode(): BehaviorSubject<EditorMode> { return this.mode; } | |||
getKeymap(): BehaviorSubject<string> { return this.keymap; } |
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.
does the keymap need to be integrated into the auto editor? it's not like users will be typing into it.
@@ -133,6 +133,11 @@ span.cm-def { | |||
background: $color-editor !important; | |||
} | |||
|
|||
.CodeMirror-dialog-bottom { | |||
position: fixed; | |||
bottom:0%; |
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.
looks like you've got some whitespace problems here: could you maybe also put a comment as to why this rule is necessary for the keymap functionality?
@@ -77,6 +77,55 @@ | |||
margin-left: 16px; | |||
} | |||
|
|||
.keymap-button { |
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.
instead of copy/pasting the mode button directly can you make parts that are actually identical into a mixin (or just one common style that both sets of elements can match directly if they actually are identical?)
{{ getPadKeymap() }} | ||
</button> | ||
<keymap-choices [@fadeInOut] *ngIf="visibleKeymapChoices != null"> | ||
<keymap-choice *ngFor="let keymap of visibleKeymapChoices" (click)="onKeymapChoice(keymap)"> |
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.
why did you ditch the is-picked
class? it makes it kinda nice to see a checkmark or whatever next to the selected one.
defaults to vim