Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#61465] users in lists miss popover additional fixes #18034

Merged
merged 4 commits into from
Feb 24, 2025

Conversation

EinLama
Copy link
Contributor

@EinLama EinLama commented Feb 21, 2025

Ticket

https://community.openproject.org/wp/61465

Additional code changes for the CKEditor can be found in commonmark-ckeditor-build#98

What are you trying to accomplish?

Fix occurrences where a hover card was expected, but did not show up:

  • in the team planner, when adding a new assignee (within the auto completer)
  • when adding a user board list
  • when mentioning users within CKEditor (dropdown selection while typing)
  • when mentioning work packages within CKEditor (dropdown selection while typing)

Additionally fix two issues:

  • the user hover card was accidentally rendered when you hover over your own avatar in the top right
  • when you hover over the avatar within another hover card, the system attempted to render another hover card on top. Since that is not possible, the existing hover card would close.

While I was at it, I fixed the way that hover card options are handled within the principal renderer. Hover cards have their own options-object now to configure whether they show up or not. Previously, this was crammed into the AvatarOptions. Since hover cards also render on usernames, it felt wrong to leave it like this.

This also allowed me to adjust the default values so that the additional two fixes from above were possible.

The work package lists two other occurrences where hover cards are missing:

  1. time and costs user filter list
  2. "move to project" user lists

Both of these were NOT added since the mentioned forms use native select elements. We cannot add popovers there without a major rework of the form.

Screenshots

team.planner.mov
CKEditor.mov
board.mov

What approach did you choose and why?

Merge checklist

  • Added/updated tests
  • Added/updated documentation in Lookbook (patterns, previews, etc)
  • Tested major browsers (Chrome, Firefox, Edge, ...)

@EinLama EinLama force-pushed the bug/61465-users-in-lists-miss-popover-additional-fixes branch from b1873e6 to b18e4d9 Compare February 24, 2025 07:47
Works for both users and work packages.
…over cards

The first feels weird. You should know who you are. The latter is even a
bug, since a hover card will close when you try to open a new one ->
prohibit this from happening.

While on it, I decided to rework the way that a hover card is configured
within the principal renderer. It now gets its own configuration object
instead of piggybacking on the avatar options. It always felt wrong to
cram the hover card options in there, too.
@EinLama EinLama force-pushed the bug/61465-users-in-lists-miss-popover-additional-fixes branch from b18e4d9 to e9f5864 Compare February 24, 2025 07:47
@EinLama EinLama marked this pull request as ready for review February 24, 2025 07:58
Copy link
Contributor

@bsatarnejad bsatarnejad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@EinLama EinLama merged commit 40056d5 into dev Feb 24, 2025
21 checks passed
@EinLama EinLama deleted the bug/61465-users-in-lists-miss-popover-additional-fixes branch February 24, 2025 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants