pleroma/pleroma-fe
0
0
Fork 0
Code Issues 343 Pull requests 27 Projects Releases Packages Wiki Activity

Disjointed popovers #2807

Closed
hj wants to merge 0 commits from gitlab-mr-iid-1540 into develop
pull from: gitlab-mr-iid-1540
merge into: pleroma:develop
Conversation 39 Commits - Files changed -
hj commented 2022-06-12 13:49:08 +00:00
Owner
Copy link
  • Separates popovers from DOM hierarchy by teleporting them to a separate container
  • Uses position: fixed to position popovers. As a result, popovers are now forcibly hidden on scroll
  • Recaclculates popover position on scroll event. Works like a charm on desktop (sub 1ms per scroll update), a bit laggy but acceptable on firefox mobile, popover vibrates but follows the scroll on mobile chrome.
  • Fixes duplicate <button> elements on react/extra buttons
  • Uses vue transition to fix animations on popups/tooltips
  • Fixes shadows so that all popovers/tooltips have proper shadows, not just status popovers
  • Replaces ugly bootleg tooltips for mention links with floating usercards (on click)

Known issues

  • hide-on-scroll only works on main document scroll, not on column scroll. Now searches for nearby .column.-scrollable and falls back to using window otherwise
  • tests are likely fucked due to changes in mentionlinks stuff? yep. fucked. fixed now
  • performance needs testing/verification
    • when i get browserstack i can test ios, i expect it to be jank af but workable.
      • actually, performance is stellar, at least best i could tell, browserstack itself is pretty laggy tho, can't test thoroughly
    • maybe add advanced option to not do that in case it causes problems?
  • ❔ maybe it's too easy now to keep hover popover open?
  • ❔ popover position doesn't update when timeline moves, but wiring that into vuex/vue events might have performance consequences
- Separates popovers from DOM hierarchy by teleporting them to a separate container - Uses `position: fixed` to position popovers. ~~As a result, popovers are now forcibly hidden on scroll~~ - Recaclculates popover position on scroll event. Works like a charm on desktop (sub 1ms per scroll update), a bit laggy but acceptable on firefox mobile, popover vibrates but follows the scroll on mobile chrome. - Fixes duplicate `<button>` elements on react/extra buttons - Uses vue transition to fix animations on popups/tooltips - Fixes shadows so that all popovers/tooltips have proper shadows, not just status popovers - Replaces ugly bootleg tooltips for mention links with floating usercards (on click) ## Known issues - [x] ~~hide-on-scroll only works on main document scroll, not on column scroll.~~ Now searches for nearby `.column.-scrollable` and falls back to using `window` otherwise - [x] ~~tests are likely fucked due to changes in mentionlinks stuff? yep. fucked.~~ fixed now - [x] performance needs testing/verification - [x] when i get browserstack i can test ios, i expect it to be jank af but workable. - actually, performance is stellar, at least best i could tell, browserstack itself is pretty laggy tho, can't test thoroughly - [ ] maybe add advanced option to not do that in case it causes problems? - :grey_question: maybe it's too easy now to keep hover popover open? - :grey_question: popover position doesn't update when timeline moves, but wiring that into vuex/vue events might have performance consequences
tusooa commented 2022-06-27 12:37:21 +00:00
Owner
Copy link

Settings modal is under top bar
image

Settings modal is under top bar ![image](/attachments/3d4d7412-f719-423b-869f-59d6fbcd6993)
image.png
32 KiB
tusooa commented 2022-06-27 12:38:54 +00:00
Owner
Copy link

Timeline selector's style did not apply
image

Timeline selector's style did not apply ![image](/attachments/3ba352df-6011-45aa-87b5-c21ce6532d40)
image.png
98 KiB
tusooa commented 2022-06-27 12:41:23 +00:00
Owner
Copy link

Clicking on the avatar here should take me to the user page (same as before), not expanding the avatar
image

um. we lost the "click to expand, click to collapse" interaction as before. I think somehow the popover should show beside the original avatar.

Clicking on the avatar here should take me to the user page (same as before), not expanding the avatar ![image](/attachments/0fbdfe74-ef1c-4a25-b391-dcf0eefadfdc) um. we lost the "click to expand, click to collapse" interaction as before. I think somehow the popover should show beside the original avatar.
image.png
380 KiB
tusooa commented 2022-06-27 12:52:13 +00:00
Owner
Copy link

This option should no longer be hidden if we are showing full usernames, as it also applies there.

This option should no longer be hidden if we are showing full usernames, as it also applies there.
tusooa commented 2022-06-27 12:53:46 +00:00
Owner
Copy link

The user popover is not showing as intended on Staff page

image

The user popover is not showing as intended on Staff page ![image](/attachments/2194d98b-3090-45fb-b7e2-05e8f2ffde27)
image.png
1 MiB
hj commented 2022-06-27 12:58:20 +00:00
Author
Owner
Copy link

oh i didn't know this was a thing

oh i didn't know this was a thing
hj commented 2022-06-27 12:58:20 +00:00
Author
Owner
Copy link

could be made an option for showing beside, sure

also could make an option for selecting what clicking the avatar in popover does, i'll default it to old behavior

could be made an option for showing beside, sure also could make an option for selecting what clicking the avatar in popover does, i'll default it to old behavior
tusooa commented 2022-06-27 13:02:45 +00:00
Owner
Copy link

it's not used on Follow Requests either

it's not used on Follow Requests either
tusooa commented 2022-06-27 13:17:46 +00:00
Owner
Copy link
  1. Use mobile layout
  2. Open notification sidebar
  3. Click on "Add reactions"
  4. Scroll
  5. The reactions popover should scroll with it, but it actually does not.
0. Use mobile layout 0. Open notification sidebar 1. Click on "Add reactions" 2. Scroll 3. The reactions popover should scroll with it, but it actually does not.
hj commented 2022-06-27 15:03:28 +00:00
Author
Owner
Copy link

couldn't reproduce this one. screenshot/browser version?

couldn't reproduce this one. screenshot/browser version?
hj commented 2022-06-27 16:54:30 +00:00
Author
Owner
Copy link

made it an option to zoom avatar when clicking it in popover, by default clicking it closes the popover, should suffice for now I think

made it an option to zoom avatar when clicking it in popover, by default clicking it closes the popover, should suffice for now I think
hj commented 2022-06-27 16:54:31 +00:00
Author
Owner
Copy link

fixed

fixed
hj commented 2022-06-27 16:54:31 +00:00
Author
Owner
Copy link

fixed BasicUserCard

fixed BasicUserCard
tusooa commented 2022-06-27 17:00:53 +00:00
Owner
Copy link

sidebar navigator style should not change
image

sidebar navigator style should not change ![image](/attachments/a424f482-3232-48ee-aa45-a79d6e4b2439)
image.png
476 KiB
tusooa commented 2022-06-27 17:02:20 +00:00
Owner
Copy link

it didn't close for me: it goes to the user page instead

it didn't close for me: it goes to the user page instead
tusooa commented 2022-06-27 17:04:10 +00:00
Owner
Copy link

firefox 101.0.1 on GNU/Linux (Wayland)
image

firefox 101.0.1 on GNU/Linux (Wayland) ![image](/attachments/54bfde3d-9963-4bc9-b508-e066cd2c29fd)
image.png
770 KiB
hj commented 2022-06-27 17:23:41 +00:00
Author
Owner
Copy link

still can't reproduce, not even with browserstack

i noticed that it doesn't close when you close sidebar however.

still can't reproduce, not even with browserstack i noticed that it doesn't close when you close sidebar however.
hj commented 2022-06-27 17:23:55 +00:00
Author
Owner
Copy link

😔

:pensive:
hj commented 2022-06-27 17:25:00 +00:00
Author
Owner
Copy link

oh so it wasn't a fluke, i wonder what's up with that.

maybe i should refactor UserCard in general

oh so it wasn't a fluke, i wonder what's up with that. maybe i should refactor UserCard in general
hj commented 2022-06-29 19:35:28 +00:00
Author
Owner
Copy link

fixed

fixed
hj commented 2022-07-20 10:05:44 +00:00
Author
Owner
Copy link

Should be fixed now

Should be fixed now
hj commented 2022-07-20 10:05:44 +00:00
Author
Owner
Copy link

still not reproducible, could be on older version of it

still not reproducible, could be on older version of it
tusooa commented 2022-07-20 21:39:41 +00:00
Owner
Copy link

i think the timeline selector dropdown should be the same size as the one in the sidebar

currently it's too small and inaccessible

i think the timeline selector dropdown should be the same size as the one in the sidebar currently it's too small and inaccessible
tusooa commented 2022-07-20 21:40:43 +00:00
Owner
Copy link

image

![image](/attachments/414913fc-3c52-4a70-8c3c-b3950b24a74b)
image.png
1.3 MiB
hj commented 2022-07-21 07:52:05 +00:00
Author
Owner
Copy link

imo the main navigation is a bit overblown but I can increase the size in dropdown.

imo the main navigation is a bit overblown but I can increase the size in dropdown.
tusooa commented 2022-07-27 20:30:13 +00:00
Owner
Copy link

I don't think we should use icon colour for text

I don't think we should use icon colour for text
tusooa commented 2022-07-27 20:34:09 +00:00
Owner
Copy link

it makes it look really weird
image

it makes it look really weird ![image](/attachments/a420e60a-5c2c-4b08-a46f-05bacdbf11fc)
image.png
246 KiB
hj commented 2022-07-27 20:50:07 +00:00
Author
Owner
Copy link

i don't really know what color to use instead to show that it's interactible, i feel like link color would be even weirder

i don't really know what color to use instead to show that it's interactible, i feel like link color would be even weirder
tusooa commented 2022-07-27 20:51:57 +00:00
Owner
Copy link

In follow requests, when you have enough ones, clicking on the last one will make the popover go out of bounds, and it's impossible to scroll down
image

In follow requests, when you have enough ones, clicking on the last one will make the popover go out of bounds, and it's impossible to scroll down ![image](/attachments/def70b51-f895-4108-a809-a56757b7541f)
image.png
229 KiB
tusooa commented 2022-07-27 20:53:14 +00:00
Owner
Copy link

i feel we should use link colour for both inactive and active

i feel we should use link colour for both inactive and active
hj commented 2022-07-27 21:06:34 +00:00
Author
Owner
Copy link

oh, overlay mode doesn't respect window's bounds, gotta fix that

oh, overlay mode doesn't respect window's bounds, gotta fix that
hj commented 2022-07-27 21:07:22 +00:00
Author
Owner
Copy link

let's keep it like this and figure it out when we refactor and redesign user profile cards, they are weird right now all over the place tbh

let's keep it like this and figure it out when we refactor and redesign user profile cards, they are weird right now all over the place tbh
tusooa commented 2022-07-27 22:44:31 +00:00
Owner
Copy link

trying to invoke user popover in a status popover will cause both to disappear

trying to invoke user popover in a status popover will cause both to disappear
tusooa commented 2022-07-27 22:56:09 +00:00
Owner
Copy link

in current develop it shows the user card inside the status popover

in current develop it shows the user card inside the status popover
hj commented 2022-07-30 21:17:25 +00:00
Author
Owner
Copy link

fixed, now popovers are nudged vertically the same way it's done horizontally, it mostly affects overlay-centers popovers tho, doesn't seem to do anything for other popovers (good)

fixed, now popovers are nudged vertically the same way it's done horizontally, it mostly affects overlay-centers popovers tho, doesn't seem to do anything for other popovers (good)
hj commented 2022-07-30 22:58:43 +00:00
Author
Owner
Copy link

Should be fixed now. I implemented a popover stacking mechanism. Unfortunately vue's custom events don't bubble and native events probably won't do much good since popovers lack parent-child relationship in the DOM. Basically, when popover is mounted it will try to find a parent popover using this.$parent repeatedly until it finds one, then when it shows or hides itself it will inform the parent that its state changed, while parents will refuse to close if any children are open. It will also call onClickOutside in parent popovers so that when you click outside all of them the entire stack (or chunk of it) closes, otherwise it would need click for each popover in stack.

It's a bit hacky, i know, realistically we'd need bubbling custom events which is whole another topic that's way outside of scope of this MR since Vue doesn't support it.

And yes, this prevents on-hover popovers from closing if they have nested (on-click or on-hover) popovers open. This is sorta necessary as if some popover in a stack closes it unmounts the nested Popover component, closing the entire tree downstream, essentially.

Should be fixed now. I implemented a popover stacking mechanism. Unfortunately vue's custom events don't bubble and native events probably won't do much good since popovers lack parent-child relationship in the DOM. Basically, when popover is mounted it will try to find a parent popover using `this.$parent` repeatedly until it finds one, then when it shows or hides itself it will inform the parent that its state changed, while _parents will refuse to close if any children are open_. It will also call `onClickOutside` in parent popovers so that when you click outside all of them the entire stack (or chunk of it) closes, otherwise it would need click for each popover in stack. It's a bit hacky, i know, realistically we'd need bubbling custom events which is whole another topic that's way outside of scope of this MR since Vue doesn't support it. And yes, this prevents on-hover popovers from closing if they have nested (on-click or on-hover) popovers open. This is sorta necessary as if some popover in a stack closes it unmounts the nested Popover component, closing the entire tree downstream, essentially.
tusooa commented 2022-07-31 00:15:29 +00:00
Owner
Copy link

confirmed to be fixed

confirmed to be fixed
tusooa commented 2022-07-31 00:39:45 +00:00
Owner
Copy link

the status popover sometimes doesn't disappear properly

image

  1. Hover on Reply to
  2. Without mouse entering the popover, hover out of Reply to
  3. on develop, the popover disappears; now it doesn't
the status popover sometimes doesn't disappear properly ![image](/attachments/9228f2c7-d3a8-4726-b206-6a2cb8728848) 0. Hover on Reply to 1. Without mouse entering the popover, hover out of Reply to 2. on develop, the popover disappears; now it doesn't
image.png
482 KiB
hj commented 2022-07-31 08:32:42 +00:00
Author
Owner
Copy link

that was a typo, fixed now.

that was a typo, fixed now.

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
BE Request

   
Backlog

   
Better behavior

   
Blocked by deps

   
Bug

   
Code Review

   
Discussion

   
Doing

   
Feature Request / Enhancement

   
In the shining bright future maybe

   
It's complicated

   
MS Edge

   
Mememoon (or other niche browsers)

   
Missing API

   
Missing feature

   
Missing l10n/i18n

   
NL1

Instance Admin Dashboard

  
NL2

Improved Mobile Experience

  
NL4

Improved instance customization and QoL options

  
Need to verify on develop

   
RELEASE BLOCKER

   
Reassign or Close

   
Refactor

   
Regression

   
Safari

   
To Do

   
User story

   
Waiting on godot

   
accessibility

https://git.pleroma.social/pleroma/pleroma-fe/-/snippets/7746

  
better documentation

   
chore

   
confirmed

   
easy ticket

   
incident

Denotes a disruption to IT services and the associated issues require immediate attention

  
l10n update

   
mastoapi

   
mobile

   
needs design

   
needs-info

   
needs-review

   
stupid
No labels
BE Request
Backlog
Better behavior
Blocked by deps
Bug
Code Review
Discussion
Doing
Feature Request / Enhancement
In the shining bright future maybe
It's complicated
MS Edge
Mememoon (or other niche browsers)
Missing API
Missing feature
Missing l10n/i18n
NL1
NL2
NL4
Need to verify on develop
RELEASE BLOCKER
Reassign or Close
Refactor
Regression
Safari
To Do
User story
Waiting on godot
accessibility
better documentation
chore
confirmed
easy ticket
incident
l10n update
mastoapi
mobile
needs design
needs-info
needs-review
stupid
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 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
pleroma/pleroma-fe!2807
No description provided.