-
Notifications
You must be signed in to change notification settings - Fork 36
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
FEATURE: Add menu to assigned text on posts #550
FEATURE: Add menu to assigned text on posts #550
Conversation
e2b9912
to
065a7f8
Compare
009d94d
to
306a155
Compare
306a155
to
a5c1cbc
Compare
a5c1cbc
to
e780f3e
Compare
e780f3e
to
f5f3475
Compare
</a> | ||
|
||
<DMenu @icon="ellipsis-h" class="btn-flat more-button"> | ||
<div class="popup-menu"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this necessary ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we need that to style the popup menu appropriately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(DMenu
doesn't provide that)
@jjaffeux, I'm ready to merge it as soon as you approve. |
approved! |
This adds a popup menu to assigned posts that makes it easier to unassign or reassign them:
It was pretty difficult to write the tests. I've been trying both acceptance and system specs and decided to stick with acceptance tests, though one acceptance spec is still not passing, and I skipped it for now. We'll be adding more similar menus in following PRs, and I'm going to invest more time into tests stability while working on that.
I haven't included the system specs I wrote into this PR for now (they don't pass) but we may decide to use them, let's see how adding other menus in following PRs will go.