Commit graph

39 commits

Author SHA1 Message Date
Bartosz Dziewoński 04cfffe323 Move visualenhancements metadata and some buttons outside of <h2>
We wrap a `<div>` tag around the `<h2>`, and move some elements there.
The markup is inspired by and compatible with my proposal for T13555.

The "ext-discussiontools-init-section" class is moved to the `<div>`.

A small patch is needed in MobileFrontend to preserve the section
collapsing functionality: I11bff21e81046898ca63f3f432797129fa70ad88.

The following elements are now outside of `<h2>`:
* Metadata bar
* Subscribe button
* Ellipsis menu (only shown on mobile)

The following elements are sadly still inside of `<h2>`:
* Subscribe links (only shown on desktop)
* Section edit links from MediaWiki core
Trying to move them mucks up the CSS too much. I hope we can resolve
this later as a part of the work on T13555.

Depends-On: I11bff21e81046898ca63f3f432797129fa70ad88
Bug: T314714
Change-Id: I0bbdcfa02c334858737855349d7a35746de1d8f2
2022-11-10 23:35:40 +01:00
Ed Sanders 79521f89cf Add button to reveal lede section on mobile
Bug: T312309
Depends-On: I9c3035c9dbe7545a05efb2286dffe0145d3557b4
Change-Id: I9d74914ddbcc9def74e85106a68574a807b0b731
2022-11-10 22:10:04 +00:00
Ed Sanders a00131a18f Embed pageThread JSON in jsConfigVars instead of infusing HTML
Bug: T322651
Depends-On: I86d461756398780dc24949013f35b7730a481052
Change-Id: I85ee8e6ed6eba97b94f4e4c415fbc5286c234cce
2022-11-08 16:20:39 +00:00
Bartosz Dziewoński 9cbb0a8c60 Don't apply topic containers to table-of-contents heading
Change-Id: I3abbf7907907f6280e6b58bcf4307c0ce3b1898e
2022-10-25 18:59:25 +00:00
Bartosz Dziewoński 8ad26730f2 CommentFormatter: Always add the overflow menu
Bug: T318110
Change-Id: I49900e32dc6b5dbca3286a840ce122639e25e9bd
2022-10-02 15:36:45 +02:00
David Lynch 4b370a8971 ThreadItem jsonSerialize: make sure callback is applied last
This changes a vast number of test expected-outputs because it affects
the order of the keys when serialized.

Bug: T315400
Change-Id: I6ad2cded6ba7cb2cc5e5ba37ea60f4b18ecc26be
2022-08-30 09:11:46 -05:00
jenkins-bot 22825f06e6 Merge "Fix subscribe button appearing for unsubscribeable sections with visualenhancements" 2022-08-25 14:59:46 +00:00
Ed Sanders 9adafd43a1 Show latest comment info in subtitle
Bug: T306675
Change-Id: I1400dbb50cdf8a0a5e23ce533c84fad96f3fae16
2022-08-23 19:31:40 +00:00
Bartosz Dziewoński 0eeffce1c1 Fix subscribe button appearing for unsubscribeable sections with visualenhancements
We didn't correctly reconcile the changes from
Ie0412b9f238d5ff8e54fd2ea358c1c26e303f4e1 and
I419883b5f9a4291b4bf575d57195d553fd5e291e.

Bug: T315821
Change-Id: I5a93cbb3bf029b352e808cbd9cb1ea3286e20c94
2022-08-23 01:07:14 +02:00
Bartosz Dziewoński a27765319b CommentFormatter: Set 'data-mw-comment' even when reply tool disabled
Move the code that generates these wrapper nodes and attributes
from postprocessReplyTool() (only called when reply tool is enabled)
to addDiscussionToolsInternal (always called).

This undoes some changes from 31c57d594a and 980b2c38bc.

Bug: T314707
Change-Id: I07ed210375d494047670015410430c087d67f21a
2022-08-06 14:16:37 +00:00
Bartosz Dziewoński 9c68e058ef CommentFormatter: Add test cases for mobile version
Also, rename the files, since CommentFormatter now does more than
replies.

Change-Id: I1ae432c06badd9790274db27881c2222c0439ba8
2022-08-02 14:21:36 +01:00
Bartosz Dziewoński 31c57d594a Do not duplicate item JSON in page HTML
Rather than setting it on both the reply link and the reply button,
set it on their parent element.

Update ReplyLinksController to handle this.

Change-Id: I650e9c0ebd354a82b8f66a63c5b4c02b2e29b105
2022-08-01 22:14:50 +00:00
Ed Sanders 980b2c38bc Make reply links into buttons when visual enhancements enabled
Bug: T255560
Bug: T309904
Change-Id: I3932f576086a43df89ff97a1b3dafdc27c54f71c
2022-08-01 20:59:53 +02:00
Ed Sanders 06d14d8c8f Visual enhancements: Fix loading of icons
* clock, userAvatar, speechBubble were dropped before topic
  containers was merged
* Load ellipsis & edit icons on mobile only
* Load bell & bellOutline for topic subscriptions button

Change-Id: I77d1336627b564be756e3ec50b4686b8f9ac72dc
2022-07-27 18:24:54 +02:00
Ed Sanders 2960853088 Move subscribe button up on desktop
Bug: T312674
Change-Id: I419883b5f9a4291b4bf575d57195d553fd5e291e
2022-07-27 13:55:03 +01:00
Ed Sanders df1cd6be80 Prepend subscribe links to headings
This ensures they stay aligned with the top of the heading when text wraps.

Bug: T313406
Change-Id: Ifd8f93f63a1b3e3e4bd38a1d74f9afed647f7e68
2022-07-27 13:48:28 +01:00
Ed Sanders 7fc5a0c29d Topic containers: Design iterations
Bug: T310914
Change-Id: I9000f9902d612c58c6b3bc8b762232ca6dd9724f
2022-06-25 12:54:39 +00:00
Ed Sanders 63acc121fc Thread containers: Link latest comment timestamp to corresponding comment
Bug: T309751
Change-Id: Id969bd7a76544d6024b8714c45cdfe5c59b71a0b
2022-06-24 21:44:21 +00:00
Ed Sanders da64c43ccc Show thread metadata in section headers
Bug: T269950
Change-Id: Ifa47ddcbccf288be0bbecd5961eab7c5122aab7b
2022-06-23 17:17:09 +01:00
Ed Sanders 0ad9b4c6b2 Move placeholder heading level (99) to a constant
Change the HeadingItem constructor to take a 'null' headingLevel
and store this internally with the constant. Change the JSON
serializer to convert this back to null.

Change-Id: I27508eed75d94b99c5189548919309f8da7deb75
2022-06-14 22:51:49 +01:00
Ed Sanders 48fdcf1056 Remove data-mw-comment-name attribute from subscribe links
The HeadingItem's name is now present in the DOM, so just traverse
to it and use that instead.

Change-Id: If28e1588742513d606e3d8fcfb259b85acc0a873
2022-03-28 22:30:59 +01:00
David Lynch 71049f4ce8 Add item name to the JSON output of HeadingItem
The name is needed for the topic subscription API

Bug: T285971
Change-Id: Iedbebdfd65d03ab01b22b35781803655749aa269
2022-03-25 11:45:50 +00:00
Bartosz Dziewoński 77614a2d02 tests: Fix root node / container handling
Since times immemorial, and for reasons lost to history, our test code
was adding an extra <div> wrapper before parsing the HTML used for
tests. This wasn't a problem, until now, because I want to add some
tests for T303396 that need to check that the *real* wrappers present
in some test cases are handled correctly.

Changes to test cases mostly remove a leading "0/" from serialized
ranges, corresponding to removing the extra wrapper.

Change-Id: Ia50e3590538c8cd274b02d2a937ba1a3fbb4ac89
2022-03-10 18:43:58 +01:00
Bartosz Dziewoński c1f4668806 Change CommentParser and ImmutableRange to use offsets in codepoints instead of bytes
The PHP DOM extension measures lengths and offsets in Unicode codepoints.
Our PHP code used UTF-8 bytes, causing some offsets to be slightly off.
Now it mostly uses Unicode codepoints as well (we're forced to use bytes
in a few places, because preg_match returns offsets in bytes).

In practice, this had no visible effect to the user. It caused the
markers `<span data-mw-comment-end="..."></span>` to be placed at
the end of their container instead of the correct position when the
timestamp contained multibyte characters (e.g. "ź" in Polish); but
the correct position is usually at the end of the container anyway.

In the test cases, the only difference is placing these markers before
a trailing line break inside `<p>...</p>` tags rather than before it.

The patch also accidentally fixes another bug, where element nodes
with no children (mostly <img>) were incorrectly excluded when calling
cloneContents(), because they were treated as if they were text nodes.

Change-Id: Iccdccf1078598f4b62cab96225e9c85a4c0e93ee
2021-09-27 19:04:16 +00:00
Bartosz Dziewoński 4ebe4b29bb Only show [subscribe] links on sections that contain at least one comment
Bug: T285796
Change-Id: I48264d55464fa6bce56b47fc1075f5d348101676
2021-07-13 02:35:15 +02:00
Bartosz Dziewoński 9c8d709b8a Use placeholder localisation messages in CommentFormatter tests
Otherwise they will fail whenever translations are updated
(and they are failing right now).

Change-Id: I849c57b86d36fb6c7739cc31a74df741e08462f4
2021-06-02 21:46:36 +02:00
Ed Sanders 6a24ceaeca Subscribe/unsubscribe with plain text links
Bug: T279149
Bug: T279151
Change-Id: Ie7d46ea2e8d458fcdad4f91bb89ba038969f6b62
2021-06-01 20:37:47 +02:00
Bartosz Dziewoński 5103e651be Add tests for CommentFormatter::postprocessTopicSubscription
Change-Id: Ief9648b8805fadcc170c54b627eb669cc8b907b6
2021-04-21 11:57:25 +00:00
Bartosz Dziewoński a3f665e816 Remove <header> tags around headings for compat with MobileFrontend
We added it because the initial designs for the subscribe action were
much easier to implement like this, and topic "containers" (T269950)
would have required it.

However, the latest design of the subscribe action will not need it
(T279149), and topic containers are still very far away, so let's
remove it for now.

Bug: T280433
Change-Id: I21a23e9bea43f24d265750926fbd62b99038d3f1
2021-04-19 17:47:43 +02:00
Ed Sanders eb7e682d2f Topic subscription front end
Bug: T276996
Change-Id: Ifb62c04c2a0ea8399749b22021d6a1c5a079bf94
2021-04-06 23:28:28 +02:00
Bartosz Dziewoński 42ce942c86 Introduce comment "names" to identify comments across revisions/pages
The existing comment IDs can't be used to find the same comment on
a different revision or page (when it's transcluded), because they
depend on the comment's parent and its position on the page.

Comment names depend only on the author and timestamp. The trade-off
is that they can't distinguish comments posted within the same minute,
or in the same edit, so we will still need the IDs sometimes.

Prefer using comment names when replying, if they're not ambiguous.
This fixes T273413 and T275821.

Heading names depend on the author and timestamp of the oldest comment.
This way we don't have to detect changes to the heading text, but we
can't distinguish headings without any comments.

Bug: T274685
Bug: T273413
Bug: T275821
Change-Id: Id85c50ba38d1e532cec106708c077b908a3fcd49
2021-03-23 16:08:42 +00:00
Ed Sanders c4de603ef9 Give comments IDs so they can be scrolled to with hash links
Bug: T265268
Change-Id: Idb985ed38bdb74e23cb7840899a61dc919f05f6f
2021-03-20 15:43:23 +00:00
Ed Sanders 4a0802065c Make IDs (to be used as URL hashes) wikitext safe
* Use hyphens instead of pipes a separators
* Use underscores for spaces in usernames

Change-Id: I6efd9739fc73e45002e50e64c43ce0de1c2f1239
2021-03-18 20:45:21 +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
Ed Sanders ccc19d8df2 Add 'href' to reply links for better compatibility with skins
Change-Id: Id948d576bbe5a6d43c4f8a06cdb2cd8ad19be193
2021-03-09 00:04:53 +01:00
Bartosz Dziewoński 5a07139249 CommentFormatterTest: Avoid re-serializing the HTML
The code we're testing already produces a string of serialized HTML,
no need to parse and re-serialize it.

Also, we recently learned that the precise format matters here
(T274709), and now this test *actually* covers the fix for that bug.
Follow-up to 5b26e9664b.

As a downside, this test might now spuriously fail if the format of
the output of Parsoid's XMLSerializer changes. Hopefully that won't
happen too often.

Change-Id: I69b514f545e47dcb437fb39a83edb8e2f19ed99b
2021-03-01 21:30:28 +01:00
Ed Sanders d05109b24d Truncate user generated part of IDs to 80 characters
This ensures that IDs fit in a 255 character database field.

Bug: T273658
Change-Id: I3cfe4fce6a865b4343f0f01121cd696aa5f98b22
2021-02-03 15:04:58 +00:00
Ed Sanders 47aea0b160 Use tabs for indentation in JSON test files
Change-Id: I1d8f8b33b19bcff249ad08dfe687f87f5e5bf9bf
2021-01-27 00:25:15 +00:00
Bartosz Dziewoński 366dc2387e Add tests covering it.wp unsigned comment template
Bug: T268178
Bug: T268589
Change-Id: Icd353275b3b8191eddbff4d72984a8c2c8e6cdd6
2020-11-25 02:08:03 +01:00