XWIKI-19751: Move the "watch" button for a page to the page content menu #3059

surli · 2024-04-15T12:33:01Z

Jira URL

https://jira.xwiki.org/browse/XWIKI-19751

Changes

Description

Goal of this PR is to simplify usage of the watch page feature.
It comes with important changes:

the watch location switches in the bell menu area have been removed
a new button with a logo and a status has been introduced next to edit button
a new modal has been introduced when clicking on the button providing various choices
the watch page API have been modified to not just provide boolean value watch / not watched, but to provide a larger panel of possible statuses
a REST resource have been provided to allow watching / blocking / removing a filter on a location

Clarifications

First important changes to review in that PR are the breaking API: they're emphasized in the revapi ignores I added. Most of them are about unstable APIs, but one of them is a real breakage I opened a brainstorming on the forum about it and I'd like feedbacks about it.

Next important change to review is the new APIs I provide:

the REST resource
the new statuses

Then only in the end the UI should be reviewed: first thing to review about the UI is the logic of the different choices available in the modal. We can easily change later where we put the button and which vocabulary we use, but if there's a flaw in the logic it's more difficult to change later at it will probably impacts the API.

Screenshots & Video

Capture.video.du.07-06-2024.10.43.38.webm

Executed Tests

Ran:

on module xwiki-platform-notifications: mvn clean install -Pquality,integration-tests,docker
on module xwiki-platform-legacy-notifications: mvn clean install -Pquality

Expected merging strategy

Prefers squash: Yes
Backport on branches: No

vmassol · 2024-04-15T12:55:56Z

and start introducing a new UIX for the watch button next to the edit button.

Did we discuss the addition of a new menu in the page content? I'm asking since this will clutter the UI more (the top menu will still be there and the net effect would be the addition of a new menu), so we need to be careful.

surli · 2024-04-15T12:58:54Z

and start introducing a new UIX for the watch button next to the edit button.

Did we discuss the addition of a new menu in the page content? I'm asking since this will clutter the UI more (the top menu will still be there and the net effect would be the addition of a new menu), so we need to be careful.

There's an open proposal on the forum for all this, but no conclusion yet. In any case this PR is a very preliminary draft: the UI will probably change but I need to see the possible actions to test and properly fix the APIs.

...pi/src/main/java/org/xwiki/notifications/filters/internal/scope/ScopeNotificationFilter.java

...tform-notifications-ui/src/main/resources/XWiki/Notifications/Code/NotificationsWatchUIX.xml

...xwiki/notifications/filters/internal/scope/ScopeNotificationFilterLocationStateComputer.java

...c/main/java/org/xwiki/notifications/filters/watch/script/NotificationWatchScriptService.java

...otifications-rest/src/main/java/org/xwiki/notifications/rest/NotificationsWatchResource.java

...t/src/main/java/org/xwiki/notifications/rest/internal/DefaultNotificationsWatchResource.java

michitux

Some comments primarily on the UI side. I think all translations deserve a closer look to make sure they're easy to understand and make sense for users.

...tform-notifications-ui/src/main/resources/XWiki/Notifications/Code/NotificationsWatchUIX.xml

michitux · 2024-06-04T09:50:30Z

...tform-notifications-ui/src/main/resources/XWiki/Notifications/Code/NotificationsWatchUIX.xml

+  #set ($ancestorRef = $ancestorFirstFilter.left)
+  #set ($ancestorWatchStatus = $ancestorFirstFilter.right)
+#end
+## FIXME: the different icons should use the icon service


This should be fixed.

Yes I haven't fixed this yet since I'm not yet 100% sure about the UI and I'd need to introduce new icons in the iconset for being able to use the icon service with current choice.

...tform-notifications-ui/src/main/resources/XWiki/Notifications/Code/NotificationsWatchUIX.xml

michitux · 2024-06-04T09:52:07Z

...tform-notifications-ui/src/main/resources/XWiki/Notifications/Code/NotificationsWatchUIX.xml

+#set ($unblockWikiOption = "#_displayOption('unblock-wiki','unblockWiki','unblockwiki','world')")
+{{html clean='false'}}
+&lt;div class="btn-group" id="watchButton"&gt;
+  &lt;a href="#" role="button" title="$escapetool.xml($watchText)" class="btn btn-default" data-toggle="modal" data-target="#watchModal"&gt;


Buttons should use a button element unless there is a problem with that.

I think I reused some existing code for that one. I'll check.

michitux · 2024-06-04T09:52:47Z

...tform-notifications-ui/src/main/resources/XWiki/Notifications/Code/NotificationsWatchUIX.xml

+{{html clean='false'}}
+&lt;div class="btn-group" id="watchButton"&gt;
+  &lt;a href="#" role="button" title="$escapetool.xml($watchText)" class="btn btn-default" data-toggle="modal" data-target="#watchModal"&gt;
+    &lt;span class="fa $watchIcon"&gt;&lt;/span&gt; $watchText


The icon should use the icon theme.

...tform-notifications-ui/src/main/resources/XWiki/Notifications/Code/NotificationsWatchUIX.xml

...xwiki-platform-notifications-ui/src/main/resources/XWiki/Notifications/Code/Translations.xml

michitux · 2024-06-04T10:01:28Z

Regarding the screenshots, I'm wondering if it is really a good idea to explain the options only after selecting them. Also, "that option" sounds strange to me, why not "this option"? Further, why is there no radio button to just not follow the page/use the inherited settings?

...-filters-watch/src/main/java/org/xwiki/notifications/filters/watch/WatchedUserReference.java

Work in progress: remove UI code for watch switch buttons, and start introducing a new UIX for the watch button next to the edit button. Start refactoring API for computing if a page is watched or not to get all info.

* Fix some tests in notification filters

* Start providing REST API for watch pages to simplify operations

* Distinguish more cases for watched / ignored pages to improve UI

* Provide implementation for REST API for watching pages * Improve WatchEntitiesManager API to support more operations * Improve WatchedEntityReference API to use UserReference

* Provide a dedicated page to handle watch settings * Start writing javascript code to handle choices

* Fix issue in WatchedEntitiesManager * Start providing doc / fixing checkstyle * Handle a bit more actions in UI

* Fix a bug in state computer related to the fact that the scoe filter hierarchy is not really a hierarchy * Provide a new API to allow computing the reference of the immediate ancestor of a page for which a filter exists * Improve UI to base possible actions on multiple criteria: current watch status, presence or not of a filter on an ancestor, and check if the page is terminal or not

* Encode all options for the watch settings modal

* Change the URL scheme for the REST API

* Minor improvments

* Provide test for notification rest module

* Fix checkstyle

* Put revapi ignores * Start fixing since

* Provide translations and improve UIX code

* Provide page object and fix integration tests

* Ensure to not display the watch status for guest

* Fix a few since and some problems in UIX

* Fix API of WatchedUserReference

* Fix bad escaping

* Better exception handling for creating references * Various improvments in translations (active voice + use of ignore) * Improve a bit the UI

* Improve UI when a page is ignored/followed by an ancestor and check proper rights

* Review available options and improve javascript to allow chaining operations: following space when the page is followed implies to first unfollow the page to not keep unnecessary filters and mess up the UI * Improve translations

* Clean up some unnecessary code

* Make the UI a bit more clear

* Fix some stuff after rebsae

* Slightly improve UX

...xwiki/notifications/filters/internal/scope/ScopeNotificationFilterLocationStateComputer.java

manuelleduc · 2024-06-11T08:27:07Z

...ilters-watch/src/main/java/org/xwiki/notifications/filters/watch/WatchedEntityReference.java

+         *
+         * @since 15.9RC1
+         */
+        NOT_SET(false, false),


No unstable there?

Hmm there's a wrong since on this one I have been fooled because of it I think.

Bad copy/paste it seems I'll fix that.

...ters-watch/src/main/java/org/xwiki/notifications/filters/watch/WatchedLocationReference.java

...ilters-watch/src/main/java/org/xwiki/notifications/filters/watch/WatchedEntityReference.java

...c/main/java/org/xwiki/notifications/filters/watch/script/NotificationWatchScriptService.java

* Fix missing unstable * Fix bad deprecated API call

surli force-pushed the feat-move-watch-button branch from 3c17e9d to 83d0716 Compare April 17, 2024 12:51

surli commented Apr 23, 2024

View reviewed changes

...pi/src/main/java/org/xwiki/notifications/filters/internal/scope/ScopeNotificationFilter.java Outdated Show resolved Hide resolved

surli force-pushed the feat-move-watch-button branch 2 times, most recently from 33d607e to 30a9cf9 Compare May 23, 2024 07:04

surli force-pushed the feat-move-watch-button branch from 76dad82 to 874aeec Compare May 30, 2024 08:29

surli marked this pull request as ready for review May 30, 2024 08:29

michitux reviewed Jun 4, 2024

View reviewed changes

...tform-notifications-ui/src/main/resources/XWiki/Notifications/Code/NotificationsWatchUIX.xml Outdated Show resolved Hide resolved

tmortagne reviewed Jun 4, 2024

View reviewed changes

...xwiki/notifications/filters/internal/scope/ScopeNotificationFilterLocationStateComputer.java Show resolved Hide resolved

tmortagne reviewed Jun 4, 2024

View reviewed changes

...c/main/java/org/xwiki/notifications/filters/watch/script/NotificationWatchScriptService.java Outdated Show resolved Hide resolved

tmortagne reviewed Jun 4, 2024

View reviewed changes

...otifications-rest/src/main/java/org/xwiki/notifications/rest/NotificationsWatchResource.java Outdated Show resolved Hide resolved

tmortagne reviewed Jun 4, 2024

View reviewed changes

...t/src/main/java/org/xwiki/notifications/rest/internal/DefaultNotificationsWatchResource.java Outdated Show resolved Hide resolved

michitux requested changes Jun 4, 2024

View reviewed changes

surli force-pushed the feat-move-watch-button branch from 14dd01c to 4059370 Compare June 4, 2024 14:18

surli commented Jun 4, 2024

View reviewed changes

...-filters-watch/src/main/java/org/xwiki/notifications/filters/watch/WatchedUserReference.java Outdated Show resolved Hide resolved

surli force-pushed the feat-move-watch-button branch from 43f9dea to 285014f Compare June 6, 2024 15:38

surli added 12 commits June 11, 2024 09:11

XWIKI-19751: Move the "watch" button for a page to the page content menu

451e72d

XWIKI-19751: Move the "watch" button for a page to the page content menu

cc069a1

* Fix some tests in notification filters

XWIKI-19751: Move the "watch" button for a page to the page content menu

72492dc

* Start providing REST API for watch pages to simplify operations

XWIKI-19751: Move the "watch" button for a page to the page content menu

dfd99a4

* Distinguish more cases for watched / ignored pages to improve UI

XWIKI-19751: Move the "watch" button for a page to the page content menu

9db5cac

* Provide implementation for REST API for watching pages * Improve WatchEntitiesManager API to support more operations * Improve WatchedEntityReference API to use UserReference

XWIKI-19751: Move the "watch" button for a page to the page content menu

40fda2f

* Provide a dedicated page to handle watch settings * Start writing javascript code to handle choices

XWIKI-19751: Move the "watch" button for a page to the page content menu

db7bbb5

* Fix issue in WatchedEntitiesManager * Start providing doc / fixing checkstyle * Handle a bit more actions in UI

XWIKI-19751: Move the "watch" button for a page to the page content menu

e11958b

* Encode all options for the watch settings modal

XWIKI-19751: Move the "watch" button for a page to the page content menu

5cc137d

* Change the URL scheme for the REST API

XWIKI-19751: Move the "watch" button for a page to the page content menu

1ca3adf

* Minor improvments

surli added 16 commits June 11, 2024 09:12

XWIKI-19751: Move the "watch" button for a page to the page content menu

87c6fde

* Provide test for notification rest module

XWIKI-19751: Move the "watch" button for a page to the page content menu

427aea2

* Fix checkstyle

XWIKI-19751: Move the "watch" button for a page to the page content menu

b09f609

* Put revapi ignores * Start fixing since

XWIKI-19751: Move the "watch" button for a page to the page content menu

c9220f2

* Provide translations and improve UIX code

XWIKI-19751: Move the "watch" button for a page to the page content menu

7c9d908

* Provide page object and fix integration tests

XWIKI-19751: Move the "watch" button for a page to the page content menu

40d3b8a

* Ensure to not display the watch status for guest

XWIKI-19751: Move the "watch" button for a page to the page content menu

448efaa

* Fix a few since and some problems in UIX

XWIKI-19751: Move the "watch" button for a page to the page content menu

25a6a41

* Fix API of WatchedUserReference

XWIKI-19751: Move the "watch" button for a page to the page content menu

85a6061

* Fix bad escaping

XWIKI-19751: Move the "watch" button for a page to the page content menu

8256074

* Better exception handling for creating references * Various improvments in translations (active voice + use of ignore) * Improve a bit the UI

XWIKI-19751: Move the "watch" button for a page to the page content menu

92edb7c

* Improve UI when a page is ignored/followed by an ancestor and check proper rights

XWIKI-19751: Move the "watch" button for a page to the page content menu

89efc19

* Clean up some unnecessary code

XWIKI-19751: Move the "watch" button for a page to the page content menu

8b1754a

* Make the UI a bit more clear

XWIKI-19751: Move the "watch" button for a page to the page content menu

314e20a

* Fix some stuff after rebsae

XWIKI-19751: Move the "watch" button for a page to the page content menu

390cb98

* Slightly improve UX

surli force-pushed the feat-move-watch-button branch from 2b0c10e to 390cb98 Compare June 11, 2024 07:25

manuelleduc reviewed Jun 11, 2024

View reviewed changes

...xwiki/notifications/filters/internal/scope/ScopeNotificationFilterLocationStateComputer.java Show resolved Hide resolved

manuelleduc reviewed Jun 11, 2024

View reviewed changes

...ters-watch/src/main/java/org/xwiki/notifications/filters/watch/WatchedLocationReference.java Outdated Show resolved Hide resolved

surli commented Jun 11, 2024

View reviewed changes

...ilters-watch/src/main/java/org/xwiki/notifications/filters/watch/WatchedEntityReference.java Show resolved Hide resolved

manuelleduc reviewed Jun 11, 2024

View reviewed changes

...c/main/java/org/xwiki/notifications/filters/watch/script/NotificationWatchScriptService.java Show resolved Hide resolved

XWIKI-19751: Move the "watch" button for a page to the page content menu

6cbee52

* Fix missing unstable * Fix bad deprecated API call

surli merged commit 5f90563 into master Jun 12, 2024
1 check passed

surli deleted the feat-move-watch-button branch June 12, 2024 07:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

XWIKI-19751: Move the "watch" button for a page to the page content menu #3059

XWIKI-19751: Move the "watch" button for a page to the page content menu #3059

surli commented Apr 15, 2024 •

edited

Loading

vmassol commented Apr 15, 2024

surli commented Apr 15, 2024

michitux left a comment

michitux Jun 4, 2024

surli Jun 4, 2024

michitux Jun 4, 2024

surli Jun 4, 2024

michitux Jun 4, 2024

michitux commented Jun 4, 2024

manuelleduc Jun 11, 2024

surli Jun 11, 2024

surli Jun 11, 2024

XWIKI-19751: Move the "watch" button for a page to the page content menu #3059

XWIKI-19751: Move the "watch" button for a page to the page content menu #3059

Conversation

surli commented Apr 15, 2024 • edited Loading

Jira URL

Changes

Description

Clarifications

Screenshots & Video

Executed Tests

Expected merging strategy

vmassol commented Apr 15, 2024

surli commented Apr 15, 2024

michitux left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michitux commented Jun 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

surli commented Apr 15, 2024 •

edited

Loading