Improvements for Link Sharing Modal #1980

New Issue

dpschen · 2022-09-14T15:55:23Z

dpschen commented

2022-09-14 15:55:23 +00:00

Copying over from vikunja/frontend#2335 (comment)

When testing the sharing functionality again I stumbled over a few issues:

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
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;

Copying over from https://kolaente.dev/vikunja/frontend/pulls/2335#issuecomment-35155 When testing the sharing functionality again I stumbled over a few issues: - [ ] 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. ![image](/attachments/1b4303a8-e358-4454-a9a3-64813374d640) - [ ] 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`. ![image](/attachments/98923d63-ee66-47dc-873d-54aaab054a30) - [ ] 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 - [ ] 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;`

dpschen referenced this issue from vikunja/frontend

2022-09-14 15:55:59 +00:00

feat(link shares): allows switching the initial view by passing a query parameter #2335

dpschen referenced this issue from vikunja/frontend

2022-09-14 15:56:49 +00:00

feat(link shares): allows switching the initial view by passing a query parameter #2335

dpschen added the

changes requested

label 2022-09-14 15:57:33 +00:00

konrad added the

kind/feature

label 2022-09-14 16:35:44 +00:00

xeruf commented

2022-09-24 23:15:45 +00:00

I guess I'll just tack this on here: Visiting something like /lists/43/settings/share creates an uncloseable popup ^^

I guess I'll just tack this on here: Visiting something like `/lists/43/settings/share` creates an uncloseable popup ^^

konrad commented

2022-09-26 12:23:58 +00:00

@xeruf Is that reproducible on try?

xeruf commented

2022-09-27 15:04:49 +00:00

well, not uncloseable there, but still unexpected: https://try.vikunja.io/list/1/settings/share
because it just says not found

however, when it points to a list not created by myself, I get this uncloseable popup

well, not uncloseable there, but still unexpected: https://try.vikunja.io/list/1/settings/share because it just says not found however, when it points to a list not created by myself, I get this uncloseable popup

konrad commented

2022-09-27 15:06:07 +00:00

That url should be /lists/1/settings/share (notice the additional s for lists).

That url should be `/lists/1/settings/share` (notice the additional `s` for `lists`).

xeruf commented

2022-09-28 10:12:51 +00:00

Ah right, I keep making that mistake. Nonetheless, try opening these in a new tab an clicking the x in the popup:
https://try.vikunja.io/lists/1/settings/edit
https://try.vikunja.io/lists/1/settings/share

It tries to navigate back, which is unexpected.
I think vikunja should check whether the previous page is on the same domain, otherwise just show the list on close.

Ah right, I keep making that mistake. Nonetheless, try opening these in a new tab an clicking the x in the popup: https://try.vikunja.io/lists/1/settings/edit https://try.vikunja.io/lists/1/settings/share It tries to navigate back, which is unexpected. I think vikunja should check whether the previous page is on the same domain, otherwise just show the list on close.

👍 1

dpschen commented

2022-09-28 10:30:54 +00:00

I think vikunja should check whether the previous page is on the same domain, otherwise just show the list on close.

I agree with this. I see this problem everywhere where we use router.back().
I'm not sure if checking the domain is the right approach. Maybe we should check if the last history entry contains the app base path instead

> I think vikunja should check whether the previous page is on the same domain, otherwise just show the list on close. I agree with this. I see this problem everywhere where we use `router.back()`. I'm not sure if checking the domain is the right approach. Maybe we should check if the last history entry contains the app base path instead

xeruf commented

2022-09-28 10:43:47 +00:00

Exactly that is what I meant :)

Furthermore, my colleague just had an issue with his jwt token being invalidated when he clicked on the link, logged in, and closed the popup - it looked like he was logged in, but all lists appeared empty. Very confusing.

Exactly that is what I meant :) Furthermore, my colleague just had an issue with his jwt token being invalidated when he clicked on the link, logged in, and closed the popup - it looked like he was logged in, but all lists appeared empty. Very confusing.

konrad commented

2022-09-28 13:38:06 +00:00

Maybe we should check if the last history entry contains the app base path instead

That sounds like a good approach. I've added an item in the backlog for this.

> Maybe we should check if the last history entry contains the app base path instead That sounds like a good approach. I've added an item in the backlog for this.

👍 1

dpschen commented

2022-11-22 11:49:57 +00:00

I try to tackle the modal navigation issues with vikunja/frontend#2735

I try to tackle the modal navigation issues with https://kolaente.dev/vikunja/frontend/pulls/2735

Sign in to join this conversation.