Page MenuHomePhabricator

Address Jade UI issues.
Closed, ResolvedPublic

Description

I noticed the following issues while reviewing the different user flows in Jade.

  • UI1. When I try to endorse a proposal that I have already endorsed, I get this error: "Could not find a proposal with matching "data" ''$1''" There should be an error that tells me I have already endorsed the proposal.
  • UI2. I can add an endorsement to two different proposals. I should not be able to endorse two different proposals. Either this should throw an error or it should move my endorsement from the old proposal to the new one. I'm partial to throwing an error.
  • UI3. The comment that is left when I delete a proposal starts with "‎jade-deleteproposal|0". Is 0 an internal identifier? If so, we could probably drop that from the comment. I'm sure at one point I thought it was necessary but I don't think it makes sense anymore.
  • UI4. "Propose a new label" button should be progressive blue when there are no proposed labels for the facet.
  • UI5. We should indicate that the user should wait somehow when performing an API action behind the UI. Maybe we could just disable the button that a user clicked while the action is happening.
  • UI6. When I try to move an endorsement and click "Endorse label" in the pop-over dialog, nothing happens. I get the following error in my console: "Cannot read property 'setLabel' of undefined at OoUiButtonInputWidget.MoveEndorsementDialog.onSubmitButtonClick" -- Upon review, this only seems to happen when I have endorsed multiple labels which should be impossible. So fixing UI2 might resolve this.
  • UI7. I should not have the option of moving other people's endorsements. It would be best if the endorsement menu had that option disabled for all but my my own endorsements.
  • UI8. Add new help text to Edit Quality popup
  • UI9. Move`damaging` icon back to error
  • UI10. Fix Move Endorsement title
  • UI11. Fix Move Endorsement confirm panel display
  • UI12. Move endorsement does not work when moving an endorsement from non-preferred label to preferred label.
  • UI13. DeleteEndorsement revision comment logs anonymous users ip address. This should default to user id 0 instead.

Event Timeline

Halfak created this task.Feb 14 2020, 9:20 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 14 2020, 9:20 PM
Halfak added a comment.EditedFeb 14 2020, 10:10 PM

Looks like the message we want for U1 is jade-nochange.

For UI2, I don't see an error message in the list. Here's my proposed en and qqq:

  • en: "jade-alreadyendorsed": "Cannot endorse $1 because you have already endorsed $2. Consider moving your endorsement instead."
  • qqq: "jade-alreadyendorsed": "An error message displayed when a user tries to endorse two different proposed labels for the same facet."
Halfak added a comment.EditedFeb 14 2020, 10:18 PM

For the help information for edit quality:

  • en: "jade-facet-editquality-desc": "What's the quality of this edit? Productive edits are good changes that should be kept. Damaging edits are unproductive changes that should be reverted. Goodfaith edits may be damaging they do not appear to be intentionally harmful. Badfaith edits (aka "vandalism") are intentionally damaging."
  • qqq: "jade-facet-editquality-desc": "A description of the meaning of productive/damaging and goodfaith/badfaith assessments. This description should highlight that it's possible to have a goodfaith damaging edit and that badfaith edits are vandalism."

Thanks for the review @Halfak. Your proposed error messages all look good to me.

I believe UI4 might be solved in @kevinbazira 's styling patchset (T242648). All others seem to be API bugs that I will fix next week.

ACraze updated the task description. (Show Details)Feb 14 2020, 10:58 PM

Change 573353 had a related patch set uploaded (by Accraze; owner: Accraze):
[mediawiki/extensions/Jade@master] Add specific help text for edit quality popup

https://gerrit.wikimedia.org/r/573353

Change 573415 had a related patch set uploaded (by Accraze; owner: Accraze):
[mediawiki/extensions/Jade@master] Fix endorse error handling

https://gerrit.wikimedia.org/r/573415

ACraze added a comment.EditedFeb 20 2020, 12:30 AM

@Halfak re: UI3 ("‎jade-deleteproposal|0"), 0 is supposed to be the number of endorsements, but seems to be broken at the moment and stuck at 0. Do we want to keep that number there? I don't really have a preference either way.

Change 573353 merged by jenkins-bot:
[mediawiki/extensions/Jade@master] Add specific help text for edit quality popup

https://gerrit.wikimedia.org/r/573353

Change 573415 merged by jenkins-bot:
[mediawiki/extensions/Jade@master] Fix endorse error handling

https://gerrit.wikimedia.org/r/573415

ACraze updated the task description. (Show Details)Feb 20 2020, 6:18 PM

Change 573652 had a related patch set uploaded (by Accraze; owner: Accraze):
[mediawiki/extensions/Jade@master] Disable dialog submit buttons on api call

https://gerrit.wikimedia.org/r/573652

Change 573661 had a related patch set uploaded (by Accraze; owner: Accraze):
[mediawiki/extensions/Jade@master] Fix propose new label button color for empty facets

https://gerrit.wikimedia.org/r/573661

Change 573704 had a related patch set uploaded (by Accraze; owner: Accraze):
[mediawiki/extensions/Jade@master] Fix multiple endorsement per user error

https://gerrit.wikimedia.org/r/573704

@Halfak re: UI3 ("‎jade-deleteproposal|0"), 0 is supposed to be the number of endorsements, but seems to be broken at the moment and stuck at 0. Do we want to keep that number there? I don't really have a preference either way.

Let's keep the number there. I think it'll be useful with filtering for edits that perform sketchy deletions. Deleting a proposal with many endorsements is probably not right.

Change 573652 merged by jenkins-bot:
[mediawiki/extensions/Jade@master] Disable dialog submit buttons on api call

https://gerrit.wikimedia.org/r/573652

Change 573661 merged by jenkins-bot:
[mediawiki/extensions/Jade@master] Fix propose new label button color for empty facets

https://gerrit.wikimedia.org/r/573661

Change 573704 merged by jenkins-bot:
[mediawiki/extensions/Jade@master] Fix multiple endorsement per user error

https://gerrit.wikimedia.org/r/573704

ACraze updated the task description. (Show Details)Feb 21 2020, 7:28 PM

Change 574102 had a related patch set uploaded (by Accraze; owner: Accraze):
[mediawiki/extensions/Jade@master] Fix endorsements count in DeleteProposal summary

https://gerrit.wikimedia.org/r/574102

ACraze updated the task description. (Show Details)Feb 21 2020, 9:54 PM

Change 574105 had a related patch set uploaded (by Accraze; owner: Accraze):
[mediawiki/extensions/Jade@master] Fix delete endorsement revision summary

https://gerrit.wikimedia.org/r/574105

Change 574109 had a related patch set uploaded (by Accraze; owner: Accraze):
[mediawiki/extensions/Jade@master] Fix MoveEndorsement dialog box title string

https://gerrit.wikimedia.org/r/574109

Change 574117 had a related patch set uploaded (by Accraze; owner: Accraze):
[mediawiki/extensions/Jade@master] Set damaging icon back to error

https://gerrit.wikimedia.org/r/574117

Change 574126 had a related patch set uploaded (by Accraze; owner: Accraze):
[mediawiki/extensions/Jade@master] Disable MoveEndorsement if not current user

https://gerrit.wikimedia.org/r/574126

Change 574102 merged by jenkins-bot:
[mediawiki/extensions/Jade@master] Fix endorsements count in DeleteProposal summary

https://gerrit.wikimedia.org/r/574102

Change 574105 merged by jenkins-bot:
[mediawiki/extensions/Jade@master] Fix delete endorsement revision summary

https://gerrit.wikimedia.org/r/574105

Change 574109 merged by jenkins-bot:
[mediawiki/extensions/Jade@master] Fix MoveEndorsement dialog box title string

https://gerrit.wikimedia.org/r/574109

Change 574117 merged by jenkins-bot:
[mediawiki/extensions/Jade@master] Set damaging icon back to error

https://gerrit.wikimedia.org/r/574117

Change 574126 merged by jenkins-bot:
[mediawiki/extensions/Jade@master] Disable MoveEndorsement if not current user

https://gerrit.wikimedia.org/r/574126

ACraze updated the task description. (Show Details)Feb 24 2020, 6:53 PM
ACraze updated the task description. (Show Details)Feb 25 2020, 12:43 AM

Change 574615 had a related patch set uploaded (by Accraze; owner: Accraze):
[mediawiki/extensions/Jade@master] Fix confirm form for MoveEndorsement dialog popup.

https://gerrit.wikimedia.org/r/574615

Change 574615 merged by jenkins-bot:
[mediawiki/extensions/Jade@master] Fix confirm form for MoveEndorsement dialog popup.

https://gerrit.wikimedia.org/r/574615

ACraze updated the task description. (Show Details)Feb 25 2020, 7:21 PM
ACraze updated the task description. (Show Details)Feb 25 2020, 10:00 PM

Change 574876 had a related patch set uploaded (by Accraze; owner: Accraze):
[mediawiki/extensions/Jade@master] Reindex endorsement arrays on move.

https://gerrit.wikimedia.org/r/574876

Change 574876 merged by jenkins-bot:
[mediawiki/extensions/Jade@master] Reindex endorsement arrays on move.

https://gerrit.wikimedia.org/r/574876

Change 575086 had a related patch set uploaded (by Accraze; owner: Accraze):
[mediawiki/extensions/Jade@master] Fix anonymous user id in DeleteEndorsement comment

https://gerrit.wikimedia.org/r/575086

ACraze updated the task description. (Show Details)Feb 26 2020, 9:34 PM

Change 575086 merged by jenkins-bot:
[mediawiki/extensions/Jade@master] Fix anonymous user id in DeleteEndorsement comment

https://gerrit.wikimedia.org/r/575086

ACraze updated the task description. (Show Details)Feb 27 2020, 8:49 PM

Ok, I believe these issues have now been addressed. All patchsets have been merged to master and should be live on beta.

Halfak closed this task as Resolved.Jun 22 2020, 4:35 PM