Improvements for Link Sharing Modal #2362

Open
opened 3 weeks ago by dpschen · 8 comments
Collaborator

Copying over from #2335

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

  • 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

  •  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 added the
changes requested
label 3 weeks ago
konrad added the
kind/feature
label 3 weeks ago

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 ^^
Owner

@xeruf Is that reproducible on try?

@xeruf Is that reproducible on try?

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
Owner

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`).

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.
Poster
Collaborator

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

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.
Owner

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.
Sign in to join this conversation.
No Milestone
No Assignees
3 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: vikunja/frontend#2362
Loading…
There is no content yet.