Page MenuHomePhabricator

[SPIKE, 8hrs] Allow recent changes to display list items in AMC mode
Closed, ResolvedPublic0 Story PointsSpike

Description

NOTE: We expect this to take a day of investigation (8hrs). Consider pairing.

Background

This is a set of open questions based on the hackday investigation and subsequent discussions

Current mockups for list items

Questions

  • To what extent can we re-use the work done on the history page to display list items for recent changes?
  • How can we ignore the table option when set using a filter on the page?
  • How can we ignore the table option when set as a user preference? (the preference currently listed as "Group changes by page in recent changes and watchlist"), i.e. can we change default preferences for mobile only?

Outcome

A comment on this task explaining next steps should be sufficient.

Developer notes

Q: How do we switch the views?
A: Query parameter in URL but also a user preference

Event Timeline

ovasileva triaged this task as High priority.May 27 2019, 3:46 PM
ovasileva created this task.
Jdlrobson renamed this task from [SPIKE] Allow recent changes to display list items in AMC mode to [SPIKE, 8hrs] Allow recent changes to display list items in AMC mode.Jul 9 2019, 4:16 PM
Jdlrobson added a project: Spike.
Jdlrobson updated the task description. (Show Details)
Restricted Application changed the subtype of this task from "Task" to "Spike". · View Herald TranscriptJul 9 2019, 4:16 PM
Jdlrobson set the point value for this task to 0.Jul 9 2019, 4:16 PM
Jdrewniak added a comment.EditedJul 16 2019, 1:09 PM

After investigating these list views today, I've come to these conclusions:

Standard list view

Despite the similarities between the action=history page and Special:RecentChanges pages, there are actually quite a few structural differences in the HTML, which will require refactoring the CSS to accommodate both pages.

The higher-level container elements like the <ul> and <li> are different between the two pages.

History pageRecent Changes
<li class="mw-tag-php7 selected before"><li class="mw-changeslist-line">
<ul id=“pagehistory"><ul class=“special">

Recommendation: There are existing mixins for these list styles in minerva.mixins.less (e.g. .changeslist-row(), .changeslist-row-item() ) These mixins can be expanded if necessary and applied to the Recent Changes page. The styles for the RecentChanges page should probably be put in the mobile.special.styles skinStyles module.

Common elements between History and Recent Changes are mostly the inline <span> elements in the list-item content:

  • .mw-changeslist-links
  • .mw-changeslist-separator
  • .mw-changeslist-date
  • .mw-diff-bytes
  • .mw-usertoollinks
  • .mw-tag-markers
  • .mw-plusminus-pos

Most of the inline elements will not need to be changed, since they are styled from the core mediawiki.special.changelis module, but some might have to be factored out of the history page and put into the mediawiki.interface.helpers.styles.less file, or into a new skinStyles override for mediawiki.special.changelist.

Grouped list view

The grouped list view is practically unworkeable on mobile. Because it uses a table layout, it's appearance cannot be modified for mobile in any reasonable way. Therefore this features has to be disabled. This means override core functionality as well as a user-preference. This approach should be a last resort, and it is. The HTML output of EnhancedChangesList.php is constructed with string concatenation, so even if we wanted to change it's structure, it would be prohibitivly time-consuming.

To disable the grouped view, we can use the FetchChangesList hook in ChangesList.php#90. In MobileFrontend, we should register this hook to do the following two things:

  1. Set $list = new OldChangesList( $context ); there is an optional second parameter, $groups, but it is only used for EnhancedChangesList
  2. return false

Given that this changes expected behaviour, if this grouped view is enabled via URL param or user preference on mobile, we should display a banner on the page, informing the user that the grouped change list is unavailable on mobile.

Oh, one more thing. The work described above is not blocked on T225499 the advanced filters work for mobile.

@Jdrewniak - not the best of news, but not too many surprises either. Do you think you can draft out a list of the tasks we would need to create? I guess one for the list view, one for hiding the group view, one for the banner (I really like the banner idea)?

Change 524031 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Disable the recent changes table-based layout on Minerva

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

Jdlrobson added a subscriber: Jdlrobson.

The grouped list view is practically unworkeable on mobile. Because it uses a table layout, it's appearance cannot be modified for mobile in any reasonable way. Therefore this features has to be disabled. This means override core functionality as well as a user-preference. This approach should be a last resort, and it is. The HTML output of EnhancedChangesList.php is constructed with string concatenation, so even if we wanted to change it's structure, it would be prohibitivly time-consuming.

I agree with this assessment. I talked to Roan today and he proposed this change as the way to do this:
https://gerrit.wikimedia.org/r/#/c/mediawiki/skins/MinervaNeue/+/524031 Disable the recent changes table-based layout on Minerva

There are existing mixins for these list styles in minerva.mixins.less (e.g. .changeslist-row(), .changeslist-row-item() ) These mixins can be expanded if necessary and applied to the Recent Changes page.

agree

The styles for the RecentChanges page should probably be put in the mobile.special.styles skinStyles module.

Sadly there's no module common to both recent changes and history.
I'd recommend 'mediawiki.special.changeslist' instead. The mobile.special.styles does not load on special pages that are not implemented in MobileFrontend and the history page is not a special page so does not get 'mediawiki.special.styles'.
[[
https://gerrit.wikimedia.org/r/#/c/mediawiki/skins/MinervaNeue/+/510135 WIP: Cleanup recent changes on Minerva (desktop) [WIP] | Here is a POC ]]

Most of the inline elements will not need to be changed, since they are styled from the core mediawiki.special.changelist module, but some might have to be factored out of the history page and put into the mediawiki.interface.helpers.styles.less file, or into a new skinStyles override for mediawiki.special.changelist.

I recommend the latter. mediawiki.interface.helpers.styles.less gets loaded in a lot more places and is specifically for those parenthetical wrapping styles. We still need to take a little care in mediawiki.special.changelist not to break other special changelist pages, but I think it's safer to be done here.

From my point of view: I think there is enough detail here to get started on the styling

Jdrewniak closed this task as Resolved.Jul 18 2019, 1:06 PM

Just created T228419 for the styling portion of this task. I think that along with T228280 encompass the work needed for this feature.