Commit graph

41 commits

Author SHA1 Message Date
Ed Sanders 185396a8e5 Merge ReplyWidget[Plain/Visual] modules
Only a fraction of a percentage of users are still using
ReplyWidgetPlain, and keeping these modules separate:
* Adds to code complexity
* Adds to ResourceLoader module bloat
* Causes bugs when we use VE dependencies in the
  core ReplyWidget class

The disadvantage is that ReplyWidgetPlain will now be
loading all of the VE dependencies, but this will make
switching to visual mode faster.

Bug: T348834
Change-Id: Ifb0cfd43fdab761c3321ad01fa9fefca26473f86
2023-10-13 11:58:12 +01:00
Bartosz Dziewoński f7d98d7690 Redo wrapper for localStorage integration
I think the issues in T329299 are caused by partially autosaved
comments. We store data in multiple localStorage keys, and if some of
them are stored but others are not (due to exceeding storage quota),
our code can't handle the inconsistent state.

We already have a wrapper around localStorage that tries to cover up
these issues. Change it so that all values specific to an instance of
a reply tool are stored under one localStorage key. This ensures that
all updates consistently succeed or fail, with no partially stored
state.

One of the reasons we haven't done this is because this requires the
whole data to be serialized to JSON every time, but our experience
with VE change 4355d697aa shows that this is fast enough.

Extra changes:
* Remove storagePrefix, now redundant
* Remove use of createConflictableStorage, now redundant
* Prefix the key with 'mw' as advised by mw.storage documentation
* Use ES6 syntax for the new code (just for fun)
* Use consistent expiry (T339042)

Bug: T329299
Change-Id: I347115f7187fd7d6afd9c6f368441e262154233b
2023-06-19 16:45:08 +02:00
Bartosz Dziewoński 9b317c6f22 ReplyWidget: Ignore pending updates after tearing down
When using ReplyWidgetVisual, this would just crashes in #getValue.

When using ReplyWidgetPlain, if the timing is right, this might update
our auto-save after the value was cleared in the teardown code,
breaking the way we restore the reply after showing new comments.

Bug: T316074
Change-Id: I23c5f17a6ff9a6ec9c73a176e4cc60e3ad96e7f1
2022-09-03 06:55:42 +02:00
Ed Sanders e2f3c2f264 ReplyWidget: Introduce a 'refresh' teardown mode that preserves auto-save
Change-Id: Ibb04f7c2813b9d3cf2bb2434a414c86eb0a7f644
2022-02-02 18:38:22 +00:00
Ed Sanders 7381d9d963 Cleanup CommentController and document
* Document all methods
* Rename comment to threadItem
* Use this.threadItem instead of passing in identical
  threadItem in various methods.
* Don't pass threadItem to ReplyWidget as we already
  pass the whole CommentController.

Change-Id: If9aad0bcf9f0e4ebf3342b75631ddac8b57f7d87
2022-02-02 00:05:19 +01:00
Ed Sanders 640638b1dc ReplyWidget: Document all methods
Change-Id: Iab509a7605c3b8e25bcc20fb7176fed23409aed4
2021-12-20 15:59:47 +00:00
Ed Sanders 900a01772f Support reply tool on mobile
Bug: T270536
Change-Id: I94d04e9cd442f9a4e0c5924da67c43a768417a8b
2021-11-03 17:49:06 +00:00
Ed Sanders f4c12e120a Define documentable types in eslintrc instead of inline
These types can be passed a parameters to any file without
creating a dependency, so it makes more sense to allow
the globally.

Change-Id: I5504465fd997b46547642e7046993b370b85586e
2021-10-17 14:38:39 +01:00
Bartosz Dziewoński 89aebfee07 New topic tool: Show empty title warning only when focussing the description
Currently the message appears too often when the user is not really
intending to interact with the widget.

Bug: T287901
Bug: T288316
Change-Id: I38b4c0b8817b9a9238fb6adc91ab2d1231650eff
2021-09-02 19:27:30 +00:00
Bartosz Dziewoński ea68e1a8f0 Introduce CommentDetails to group up and document stuff we pass around
Change-Id: I809524ea8d89130548964ba9e5a73f00f6089ff4
2021-06-21 22:12:18 +02:00
Ed Sanders 1893405635 Code style: Move var declarations inline
Change-Id: I1686603388b050ba4ec22eff23e4806cdf262b87
2021-04-22 17:43:46 +00:00
Bartosz Dziewoński 566d4d92db Trim wikitext for preview
And change some uses of trim() to htmlTrim() for consistency.

Bug: T276612
Change-Id: I1f55f4793af8f3dda597288c9e3427abe31c37c9
2021-03-24 18:53:41 +01:00
Bartosz Dziewoński 9da3949c47 Add accessibility labels to a few controls
Topic title field and old wikitext reply field had only placeholders,
but no labels.

Mode selector had labels on individual items, but not on the main
control.

Change-Id: I422e7e5baa8711340a1bb82255e788f2272c45c9
2021-03-18 15:15:40 +01:00
Ed Sanders ece8ff69ff Change dt- class prefix to ext-discussiontools-
Longer, but follows the style guide and less likely to conflict.

We need to account for init classes in the cache being around for
a while.

Change-Id: I738bc93393850db320fdbda2b003ca8ac40556da
2021-03-13 14:42:39 +00:00
Bartosz Dziewoński 48d5a67d41 ReplyWidget: Pass initial config values to #setup, not to constructor
The same ReplyWidget instance can be re-used when the user cancels
replying and then starts replying again to the same comment.

Previously, the initial values passed to the constructor (when switching
modes) would still persist in this case, even if they were incorrect.

Bug: T263061
Change-Id: I3dff007456bfd4f3149c718ae72fbf373a2e2913
2020-09-25 20:00:00 +02:00
Ed Sanders ebaa132ebf Edit summary in advanced mode
Bug: T249391
Change-Id: Ibfd1cf77293d30b8e0d9869bbdc9aae2aa830a06
2020-09-14 22:29:54 +01:00
Ed Sanders ebe2071dfd Upstream setPending to ReplyWidget
Now consistently uses setReadOnly instead of setDisabled.

Slight styling regression was fixed in I01e11c2ed.

Change-Id: Ice7ce00929ff9a53d0fb533f1094e7324749f3a4
2020-09-09 19:23:34 +00:00
Ed Sanders c32a8996c7 Fix reply widget documentation
Change-Id: I46d033595ecf6c226ae23aefb03884841a3299c7
2020-08-19 20:05:55 +00:00
Bartosz Dziewoński b706eac8bc Re-apply new reply API patches (again)
This reverts commit 4d7c98b97c.

Change-Id: I4100521efb687ec324d25e273a9c986fd5dac0d0
2020-08-19 20:05:42 +00:00
Bartosz Dziewoński 4d7c98b97c Revert new reply API (again)
Causes page corruption, in a new way we haven't seen before.

* Revert "Move page updating logic to controller.js"
  This reverts commit 54fdc6de06.
* Revert "ReplyWidget: Move clear methods from #teardown to #clear"
  This reverts commit 9b811a94e0.
* Revert "ApiDiscussionToolsEdit: Do not pass 'basetimestamp'"
  This reverts commit 7de5938a6f.
* Revert "Use DOMCompat::getOuterHTML instead of doc->saveHTML()"
  This reverts commit 7b2448d2f0.
* Revert "CommentController: Remove remains of client-side edit conflict handling"
  This reverts commit 2d038af705.
* Revert "Restore error message for when comment is deleted while replying"
  This reverts commit 655c0526d6.
* Revert "Use transcluded from API to avoid ever fetching Parsoid DOM in client"
  This reverts commit 9d0fc184fe.
* Revert "Create a 'transcludedfrom' API endpoint"
  This reverts commit 5d8f3b9051.
* Revert "Edit API for replies"
  This reverts commit 8829a1a412.

Bug: T259855
Change-Id: I6419408c6194ec0afa6b8ee604b12c1a24c6ac7b
2020-08-13 20:19:29 +02:00
Ed Sanders 9b811a94e0 ReplyWidget: Move clear methods from #teardown to #clear
Change-Id: Ieb02f57d31579ac9c3ee2f20f07f9107e70348d4
2020-08-13 11:11:08 +01:00
Esanders c26ca107db Re-apply new reply API patches
This reverts commit 96953647c3.

* Re-apply "Edit API for replies"
  This applies commit 8829a1a412.
* Re-apply "Create a 'transcludedfrom' API endpoint"
  This applies commit 5d8f3b9051.
* Re-apply "Use transcluded from API to avoid ever fetching Parsoid DOM in client"
  This applies commit 9d0fc184fe.
* Re-apply "Restore error message for when comment is deleted while replying"
  This applies commit 655c0526d6.

Change-Id: Id20d21899f87464636022aa0683f8c03e0060117
2020-08-07 21:31:38 +02:00
Bartosz Dziewoński 96953647c3 Revert new reply API
Causes page corruption.

* Revert "Restore error message for when comment is deleted while replying"
  This reverts commit 655c0526d6.
* Revert "Use transcluded from API to avoid ever fetching Parsoid DOM in client"
  This reverts commit 9d0fc184fe.
* Revert "Create a 'transcludedfrom' API endpoint"
  This reverts commit 5d8f3b9051.
* Revert "Edit API for replies"
  This reverts commit 8829a1a412.

Bug: T259855
Change-Id: I98036f14dd900b51f20e98696e31b9b618eceee1
2020-08-07 18:18:21 +02:00
jenkins-bot 956039e7e2 Merge "Use transcluded from API to avoid ever fetching Parsoid DOM in client" 2020-07-27 23:56:15 +00:00
Ed Sanders 9d0fc184fe Use transcluded from API to avoid ever fetching Parsoid DOM in client
Bug: T252558
Depends-On: I015ac183a0c25dafb9b95c577edd4ef59c112d43
Change-Id: I6090c99429bf6eceb57870e3d449de6754063353
2020-07-28 00:50:15 +02:00
Ed Sanders 981d3326aa Add ime-position-inside ULS class to reply widgets
Bug: T255191
Depends-On: Ibcf7a4f076ceb86b91e81310bcb2fa64abc551dd
Change-Id: I0729933551cf5b73ca99718d54635caefdd0ee94
2020-07-15 18:27:02 +01:00
David Lynch c92fad4fe7 Track more events for VisualEditorFeatureUse
* Track editor mode switching between visual and source (plain)
* Track interactions with checkboxes
* Fix the tracking of firstChange when switching with content

Bug: T254291
Change-Id: I0e243e393b02952684f34dbbe1697633c0b2561a
2020-06-04 12:19:46 -05:00
Ed Sanders 9fbafef652 Add autosave support to visual mode
Bug: T240257
Change-Id: I87f8b68a5afee8d03c81cb283c9bee68ef9d4a5d
2020-05-20 22:16:20 +01:00
Ed Sanders d56b512f2e ReplyWidget: Move all events bind to setup
Bug: T252439
Change-Id: I3c76e6364b63679fcbe40d256d9ba0c3eaba1578
2020-05-17 21:12:23 +01:00
Ed Sanders ea0b95be05 Manually clear out session storage on teardown
We used to rely on this happening in onInputChange but
that listener is now detached.

Bug: T252176
Change-Id: I6654d68dc6107cd78172c455db9cba6cb9eabddf
2020-05-08 23:01:17 +01:00
Ed Sanders 56d8721fd7 Switching between VE & wikitext (plain)
Bug: T234403
Change-Id: I84a639ab8e31ec9f683a63b0ecf184f426df09dd
2020-05-06 18:03:36 +01:00
Ed Sanders a5ed469843 Create CommentController class and move most Controller methods to it
This means we have an instance for each comment, which makes keeping
track of local variables much simpler, and will make switching out
the widgets easier.

Change-Id: I7584870225b1a14146852987d6a40ad05957387c
2020-04-29 22:21:32 +02:00
Ed Sanders 04f8e21554 Implement auto-save for ReplyWidgetPlain
Bug: T240257
Change-Id: Ica51fb51a5091b11aceb3cd5c484ef9c71b98af2
2020-04-28 20:32:20 +02:00
Ed Sanders 5597787eb4 ReplyWidget: Replace .mode with #getMode
.mode was only set the end of the constructor.

Change-Id: I1a14167d1b2e524742da31f78c6cbfaf96aec667
2020-04-14 19:42:29 +00:00
Ed Sanders 23966cc098 Move wikitext comment building to the controller
Change-Id: I6374ea570f093ae5286c14ed49a9be4f32f23ff5
2020-03-09 12:52:06 +00:00
David Lynch fb006e6373 Instrumentation
Bug: T243364
Change-Id: I8573993db0dad408f09202e548206b009c106cc9
2020-02-24 19:50:27 +01:00
jenkins-bot 95a663ff62 Merge "ReplyWidget: Handle save errors" 2020-01-29 20:51:50 +00:00
Bartosz Dziewoński 890588f36a Pick reply insertion point based on parser tree, not DOM tree
I don't like that I had to special-case `<p>` tags (top-level
comments) in this code. I feel like it should be possible to handle
top-level comments and replies in a generic way, but I couldn't find
a way to do it that actually worked.

Notes about changes to the behavior, based on the test cases:

* Given a top-level comment A, if there was a "list gap" in the
  replies to it: previously new replies would be incorrectly added at
  the location of the gap; now they are added after the last reply.
  (T242822)

  Example: "pl", comment at "08:23, 29 wrz 2018 (CEST)"

* Given a top-level comment A and a reply to it B that skips an
  indentation level: previously new replies to A would be added with
  the same indentation level as B; now they are added with the
  indentation level of A plus one. (The old behavior wasn't a bug, and
  this is an accidental effect of other changes, but it seems okay.)

  Example: "pl", comment at "03:22, 30 wrz 2018 (CEST)"
    and reply at "09:43, 30 wrz 2018 (CEST)"

* Given a top-level comment A, a reply to it B, and a following
  top-level comment C that starts at the same indentation level as B:
  previously new replies to A would be incorrectly added in the middle
  of the comment C, due to the DOM list structure; now they are added
  before C. (T241391)

  (It seems that comment C was supposed to be a multi-line reply that
  was wrongly indented. Unfortunately we have no way to distinguish
  this case from a top-level multi-line comment that just happens to
  start with a bullet list.)

  Example: "pl", comments at "03:36, 24 paź 2018 (CEST)",
    "08:35, 24 paź 2018 (CEST)", "17:14, 24 paź 2018 (CEST)"

* In the "en" example, there are some other changes where funnily
  nested tags result in slightly different results with the new code.
  They don't look important.

* In rare cases, we must split an existing list to add a reply in the
  right place. (Basically add `</ul>` before the reply and `<ul>`
  after, but it's a bit awkward in DOM terms.)

  Example: split-list.html, comment "aaa"; also split-list2.html
    (which is the result of saving the previous reply), comment "aaa"

* The modifier can no longer generate DOM that is invalid HTML, fixing
  a FIXME in modifier.test.js (or at least, it doesn't happen in these
  test cases any more).

Bug: T241391
Bug: T242822
Change-Id: I2a70db01e9a8916c5636bc59ea8490166966d5ec
2020-01-23 21:13:12 +01:00
Bartosz Dziewoński 6d243ed74a ReplyWidget: Handle save errors
Bug: T240519
Depends-On: Ie18666b41f4aff1ab4bcf93f9df6e3000ac7b500
Depends-On: I2a731cb273401074e65f9283c1f629dbdb272002
Change-Id: Ice92fafb1f546510dab28e3f8aa7d2280668965a
2020-01-21 22:21:57 +01:00
Ed Sanders e83af3a4e9 Allow plain reply widget to grow without limit
Matches the behaviour of the VE widget.

Change-Id: If6540c4e91566da878c95b40e98ebe5f996125ce
2020-01-21 19:30:34 +00:00
Ed Sanders 2f1cf65233 Option to integrate VisualEditor instead of textarea
* Add config option $wgDiscussionToolsUseVisualEditor (default false).
* Add new modules ext.discussionTools.ReplyWidgetPlain and ...ReplyWidgetVisual,
  replacing ...ReplyWidget. Load only one of them depending on the config.

TODO:
* Also add the visual mode of VisualEditor, this only uses NWE now.
  There is already code to support saving from it, but no mode
  switcher tool

Co-Authored-By: Ed Sanders <esanders@wikimedia.org>
Co-Authored-By: Bartosz Dziewoński <matma.rex@gmail.com>
Change-Id: I9b6db865d51baf400fb715dc7aa68ccd8cdd4905
2020-01-07 22:15:03 +00:00