feat(link shares): allows switching the initial view by passing a query parameter #2335
No reviewers
Labels
No Label
area/internal-code
changes requested
confirmed
dependencies
duplicate
good first issue
help wanted
hosting
invalid
kind/bug
kind/feature
question
wontfix
No Milestone
No project
No Assignees
4 Participants
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: vikunja/frontend#2335
Loading…
Reference in New Issue
No description provided.
Delete Branch "feature/redirect-to-specific-view"
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?
This PR introduces the
view
query parameter to be appended at the end of any link share. If provided, the link share will automatically switch to the view passed in the query, assuming it is one oflist
,gantt
,table
orkanban
.It also cleans up the link share table a bit as it became too crowded.
Before:
After:
Hi konrad!
Thank you for creating a PR!
I've deployed the changes of this PR on a preview environment under this URL: https://2335-feature-redirect-to-specific-vie--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 somehwere. The easiest to use is https://try.vikunja.io/.
Have a nice day!
@ -83,3 +83,1 @@
<th>{{ $t('list.share.attributes.name') }}</th>
<th>{{ $t('list.share.attributes.sharedBy') }}</th>
<th>{{ $t('list.share.attributes.right') }}</th>
<th> </th>
Why is the
necessary?I thought it was necessary, looks like it isn't.
@ -207,6 +227,12 @@ const showDeleteModal = ref(false)
const linkIdToDelete = ref(0)
const showNewForm = ref(false)
interface SelectedViewMapper {
Use
Record
withIList['id']
.I think in the future even if our ids are defined as numbers in the api it might make sense to cast them as strings in the frontend.
I got the idea while reading the zod documentation.
See: https://github.com/colinhacks/zod#record-key-type under A note on numerical keys.
We should define the types of possible views only once.
Something like:
I'm not so good in the use of TS enums yet but I have heard quite often hat using objects / arrays with
as const
and then getting the types from them can have advantages. E.g. if I remember corretly iteration is easier.So as a result:
We can then also define the options like this:
Makes a lot of sense. I knew there had to be a way to do this properly :)
I've called it
ListView
though because this is about lists and not directly about tasks.@ -257,3 +287,2 @@
function getShareLink(hash: string) {
return frontendUrl.value + 'share/' + hash + '/auth'
function getShareLink(hash: string, view: string = 'list') {
Use then the
TaskView
type here.Done.
@ -75,3 +75,3 @@
password: password.value,
})
router.push({name: 'list.list', params: {listId}})
let view = 'list'
Use the
TaskView
type.We can then check with:
Done.
Where did the 'Name' column go?
I recently shared some assets via Nextcloud and was really happy when I could share the same folder with different links and create a name so that I am able to separate them.
Sideidea:
Maybe it would be cool we should even define a stupid default name.
Something like this onky vijunja-flavoured https://github.com/Atrox/haikunatorjs
8831c83261
to7a457eb161
The column is gone, but if the link has a name it will still be shown. It then looks like this:
The shre in my screenshots didn't have a name so nothing showed up.
@ -140,2 +146,2 @@
{{ $t('list.share.right.read') }}
</template>
<div class="select">
<select v-model="selectedView[s.id]" id="linkShareView">
The id seems unused
Removed it. Was a leftover from an earlier version.
@ -141,1 +146,3 @@
</template>
<div class="select">
<select v-model="selectedView[s.id]" id="linkShareView">
<option value="list">
Create the options with a computed base on the LIST_VIEWS. This way the options will always stay up-to-date in case we add a new view.
Done.
We should try to prevent dynamic message compilation.
See: https://vue-i18n.intlify.dev/guide/advanced/optimization.html#optimization
That's what I meant here: #2335 (comment)
How would we do this then? Compute an object with the final messages?
Sorry overread your answer.
Now thinking about it we cannot fullfill both goals (making the type of views update automatically and prevent dynamic message compilation). I would opt for what I said initially. Having a configurable option map:
By making the key from type
TaskView
we prevent at least misuse.We need to rewrite this to a computed though so that it updates when the language changes.
Here's what I came up with:
e67fc7fb7e
That might not be the most ideal solution but is seems to work pretty good.
I think that’s the way to go.
Picky: use an explaining variable name for the label and key in the template
EDIT:
I created a new issue with these tasks
When testing the sharing functionality again I stumbled over a few other points:
Unsure: move the 'Create a new link share'-Button and the create-form below the list of already created shares. This way it's clear that the newly created share is added to the bottom of the list. Possible downside: out of view on mobile and small desktop windows.
Non reversible UI action: Clicking the 'Create a new link share'-Button should be reversible if you clicked there by mistake. Aka: add a 'Cancel' or 'x' button in the inline form.
Inline form the create a new share link is hard to tell apart from the rest of the form.
Missing space between 'Cancel' and 'Create' button. This might be a sideeffect from some of my Modal changes, but might make sense to look into this here. Seems like a
gap
property is missing for.card-footer
.When clicking the copy button it should be indicated that the new link is now in the clipboard. E.g. marking the copy button green and adding checkmark or success message. Would prefer the checkmark since this way the user doesn't loose context and the feedback is immediately and inline. After a few seconds the button should go back in its initial state. This button or maybe the whole input should be abstracted dedicated component
Long share names don't look good. Especially the input form gets too small. It seems fine for me that you can only see the link partly but only showing
https
seems to be not enough. Maybe we should put the share name in a new line above the rest of the info, since we never know its length.This is a general issue on mobile: It's hard to make the tooltip of 'What is a share link?' appear. Tapping makes it appear but it will directly disappear again. Since the info is so short I think we can remove the tooltip here completely and simply show the explaining text.
Picky: The tap area of the copy button seems to small on mobile. It would benefit from an
aspect-ratio: 1/1;
Related: #2361
Those are all good points! However as they are not all related to the changes in this PR I think we should do a follow-up PR with them in order to keep this PR small and reviewable. Maybe in combination with #2361.
IHMO only the 6th point on the list (long names) is something related to the changes I made here. The other ones are problems in the current dev release as well.
My quick and easy fix for the problem with too long links is this:
I also made the name italic so that it is easier to differentiate from the other info.
Moved to new issue instead :)
Thank you for implementing this! New Release? :)
There's a few more things I'd like to bundle up for the next release.