vikunja
/
vikunja
6
43
Fork 71
Code Issues 150 Pull Requests 18 Packages Releases 32 Activity

Frontend: Description Edit Hotkeys #2265

Open
Elscrux wants to merge 3 commits from Elscrux/vikunja:feature/description-edit-hotkeys into main
pull from: Elscrux/vikunja:feature/description-edit-hotkeys
merge into: vikunja:main
Conversation 2 Commits 3 Files Changed 3 +42 -2
Elscrux commented 2024-04-10 22:42:25 +00:00
Contributor
Copy Link

Adds two new hotkeys which make it much quicker and more intuitive to work with the description.
Double click on the description enters edit mode, and escape discards all current edits and exit edit mode.

Adds two new hotkeys which make it much quicker and more intuitive to work with the description. Double click on the description enters edit mode, and escape discards all current edits and exit edit mode.
Elscrux added 2 commits 2024-04-10 22:43:01 +00:00
continuous-integration/drone/pr Build is passing Details
0c08db9195 Cancel current edits and exit edit mode with escape
frederick commented 2024-04-10 23:10:10 +00:00
Member
Copy Link

Hi Elscrux!

Thank you for creating a PR!

I've deployed the frontend changes of this PR on a preview environment under this URL: https://2265-feature-description-edit-hotkeys--vikunja-frontend-preview.netlify.app

You can use this url to view the changes live and test them out.
You will need to manually connect this to an api running somewhere. The easiest to use is https://try.vikunja.io/.

This preview does not contain any changes made to the api, only the frontend.

Have a nice day!

Beep boop, I'm a bot.

Hi Elscrux! Thank you for creating a PR! I've deployed the frontend changes of this PR on a preview environment under this URL: https://2265-feature-description-edit-hotkeys--vikunja-frontend-preview.netlify.app You can use this url to view the changes live and test them out. You will need to manually connect this to an api running somewhere. The easiest to use is https://try.vikunja.io/. This preview does not contain any changes made to the api, only the frontend. Have a nice day! > Beep boop, I'm a bot.
konrad reviewed 2024-04-11 15:30:51 +00:00
frontend/src/components/input/editor/TipTap.vue Outdated
@ -67,6 +67,7 @@
class="tiptap__editor"
:class="{'tiptap__editor-is-edit-enabled': isEditing}"
:editor="editor"
@dblclick="setEditIfApplicable()"
konrad commented 2024-04-11 15:30:48 +00:00
Owner
Copy Link

Where is this event defined?

Where is this event defined?
Elscrux commented 2024-04-26 14:47:48 +00:00
Author
Contributor
Copy Link

It seems to be a native event, just like @click

It seems to be a native event, just like @click
👍 1
Elscrux marked this conversation as resolved
frontend/src/components/input/editor/TipTap.vue Outdated
@ -461,2 +477,3 @@
bubbleNow()
emit('save', editor.value?.getHTML())
lastSavedState = editor.value?.getHTML() ?? ''
emit('save', lastSavedState)
konrad commented 2024-04-11 15:28:21 +00:00
Owner
Copy Link

Why the extra variable?

Why the extra variable?
Elscrux commented 2024-04-26 14:50:43 +00:00
Author
Contributor
Copy Link

For example when you type something in the editor, I didn't see anything (other than requesting the backend) to get the state before you typed something. This is what I store in this variable, the unmodified state of the editor which we want to return to when hitting escape and discard all changes.
If there is an easier way to do it, please let me know. I was just trying things out.

For example when you type something in the editor, I didn't see anything (other than requesting the backend) to get the state before you typed something. This is what I store in this variable, the unmodified state of the editor which we want to return to when hitting escape and discard all changes. If there is an easier way to do it, please let me know. I was just trying things out.
konrad commented 2024-04-26 15:16:40 +00:00
Owner
Copy Link

I was mainly talking about syntax here. What about inlining the variable like this:

emit('save', editor.value?.getHTML() ?? '')
I was mainly talking about syntax here. What about inlining the variable like this: ``` emit('save', editor.value?.getHTML() ?? '')
konrad commented 2024-04-26 15:18:59 +00:00
Owner
Copy Link

As far as I see, the variable will only ever have the value you set it to here when it is emitted.

As far as I see, the variable will only ever have the value you set it to here when it is emitted.
Elscrux commented 2024-04-26 15:19:14 +00:00
Author
Contributor
Copy Link

Oh, that is because lastSavedState is also used in exitEditMode.
The save function updates this state and the exit function restores this state.

Oh, that is because `lastSavedState` is also used in exitEditMode. The save function updates this state and the exit function restores this state.
konrad commented 2024-04-26 15:34:16 +00:00
Owner
Copy Link

Okay, but isn't this duplicated then? Couldn't we call bubbleSave in that function?

Okay, but isn't this duplicated then? Couldn't we call `bubbleSave` in that function?
Elscrux commented 2024-04-26 15:35:14 +00:00
Author
Contributor
Copy Link

No, the idea is that the current edits are discarded and the previous state is returned. bubbleSave would save the current edits instead.

No, the idea is that the current edits are discarded and the previous state is returned. `bubbleSave` would save the current edits instead.
konrad commented 2024-04-26 15:36:21 +00:00
Owner
Copy Link

Ahh gotcha!

The use of a variable instead of a ref here is a bit unsual but I'd say it's fine as long as it works.

Ahh gotcha! The use of a variable instead of a ref here is a bit unsual but I'd say it's fine as long as it works.
👍 1
Elscrux commented 2024-04-26 15:37:16 +00:00
Author
Contributor
Copy Link

Ok :) I wasn't sure how it's done properly, so I'm happy to refactor it if there is a better way to do it.

Ok :) I wasn't sure how it's done properly, so I'm happy to refactor it if there is a better way to do it.
frontend/src/components/input/editor/TipTap.vue Outdated
@ -467,1 +484,4 @@
function exitEditMode() {
editor.value?.commands.setContent(lastSavedState, false)
if (isEditing.value) {
konrad commented 2024-04-11 15:29:55 +00:00
Owner
Copy Link

How does this behave when editing a project description? There the editor is not used as a rendering engine, so it should not change the mode to preview.

How does this behave when editing a project description? There the editor is not used as a rendering engine, so it should not change the mode to preview.
Elscrux commented 2024-04-26 15:01:54 +00:00
Author
Contributor
Copy Link

Currently it changes it back to the preview mode, which is not what it should be.
I'll add an opt-in feature for this, or what do you think should be done for this?

Currently it changes it back to the preview mode, which is not what it should be. I'll add an opt-in feature for this, or what do you think should be done for this?
konrad commented 2024-04-26 15:20:53 +00:00
Owner
Copy Link

Iirc there are props already controlling if it will allow rendering or editing, maybe use those? (Not on my pc right now, so I'm unable to check this in detail)

Iirc there are props already controlling if it will allow rendering or editing, maybe use those? (Not on my pc right now, so I'm unable to check this in detail)
Elscrux commented 2024-04-26 15:27:30 +00:00
Author
Contributor
Copy Link

There is isEditEnabled, but it is enabled by default for something like editing the project description as well.
I see that showSave is only used by the two editors, for comments and task description which should get this feature. So functionally the discard feature could be baked into this, but I'm not sure if that makes a lot of sense.

There is `isEditEnabled`, but it is enabled by default for something like editing the project description as well. I see that `showSave` is only used by the two editors, for comments and task description which should get this feature. So functionally the discard feature could be baked into this, but I'm not sure if that makes a lot of sense.
konrad commented 2024-04-26 15:35:16 +00:00
Owner
Copy Link

Good point. Let's add a new prop previewAfterExit or something similar and then refactor it later.

Good point. Let's add a new prop `previewAfterExit` or something similar and then refactor it later.
Elscrux force-pushed feature/description-edit-hotkeys from 0c08db9195 to 0e6fea95cb 2024-04-26 17:09:09 +00:00 Compare
Elscrux commented 2024-04-26 17:09:37 +00:00
Author
Contributor
Copy Link

@konrad Let me know if this works :)

@konrad Let me know if this works :)
Some checks failed
continuous-integration/drone/pr Build is failing
Required
Details
This pull request doesn't have enough approvals yet. 0 of 1 approvals granted.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
dependencies

   
duplicate

This issue or pull request already exists

  
help wanted

Need some help

  
invalid

Something is wrong

  
kind/bug

Something is not working

  
kind/feature

New feature

  
needs reproduction

   
question

More information is needed

  
security

   
wontfix

This won't be fixed

No Label
dependencies
duplicate
help wanted
invalid
kind/bug
kind/feature
needs reproduction
question
security
wontfix
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: vikunja/vikunja#2265
No description provided.
Delete Branch "Elscrux/vikunja:feature/description-edit-hotkeys"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?