Redesigned history page comparison#12775
Conversation
|
@lokesh I was unable to properly look into the silent fallback logic since pagination on the local dev environment does not seem to be working |
for more information, see https://pre-commit.ci
|
Thanks for the PR, @Saad259. 🤖 Copilot has been assigned for an initial review. @lokesh is assigned to this PR and currently has:
Possible improvements for this PR
PR triage checklist (maintainers / Pam)
Note This comment was automatically generated by Pam, Open Library's Project AI Manager, on behalf of @mekarpeles. Pam is designed to provide status visibility, perform basic project management functions and relevant codebase research, and provide actionable feedback so contributors aren't left waiting. |
There was a problem hiding this comment.
Pull request overview
This PR updates Open Library’s page history UI (e.g. ?m=history) to better support common diff workflows by adding per-revision diff links, refreshing the table layout, and introducing client-side logic for arbitrary A/B comparisons.
Changes:
- Redesign
history.htmltable structure to present revision info, per-row diff actions, and A/B selection controls. - Add new
history.jsmodule (lazy-loaded from the main JS entrypoint) to generate an arbitrary-compare action when A and B are selected. - Add new CSS styling for the redesigned history table and badges, plus update i18n message extraction accordingly.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| static/css/components/page-history.css | Adds styling for redesigned history table (current-row highlight, badges, diff links, arbitrary-compare button). |
| openlibrary/templates/history.html | Reworks history table markup to the new “Edit / Diff / Pick (A/B)” layout and adds per-row diff links. |
| openlibrary/plugins/openlibrary/js/index.js | Lazy-loads the new history comparison script when #pageHistory is present. |
| openlibrary/plugins/openlibrary/js/history.js | Implements client-side “arbitrary A/B compare” link injection based on radio selections. |
| openlibrary/i18n/messages.pot | Updates extracted strings to reflect new/changed history page UI text. |
Comments suppressed due to low confidence (1)
openlibrary/templates/history.html:89
- The form still includes A/B radio inputs and a hidden
m=diff, but there is no non-JS submit control anymore. With JavaScript disabled or if the dynamic import fails, users can’t perform an arbitrary A/B comparison. Consider restoring a real submit button (e.g., “Compare”) as a progressive-enhancement fallback (JS can still inject/relocate a link/button as desired).
</tbody>
</table>
<input type="hidden" name="m" value="diff"/>
</form>
| const href = `?m=diff&a=${lowerRev}&b=${higherRev}`; | ||
|
|
||
| const link = document.createElement('a'); | ||
| link.href = href; | ||
| link.textContent = `Compare ${lowerRev} with ${higherRev}`; | ||
| link.className = 'compare-arbitrary-btn'; |
| const radios = pageHistoryElement.querySelectorAll('input[name="a"], input[name="b"]'); | ||
|
|
||
| function updateCompareButton() { | ||
| const checkedRadioA = pageHistoryElement.querySelector('input[name="a"]:checked'); | ||
| const checkedRadioB = pageHistoryElement.querySelector('input[name="b"]:checked'); | ||
|
|
|
|
||
| // History page comparison | ||
| const pageHistory = document.querySelector('#pageHistory'); | ||
| if (pageHistory){ |
| export function initHistory(pageHistoryElement){ | ||
| console.log('initHistory called', pageHistoryElement); | ||
| console.log('pre-checked a:', pageHistoryElement.querySelector('input[name="a"]:checked')); | ||
| console.log('pre-checked b:', pageHistoryElement.querySelector('input[name="b"]:checked')); | ||
|
|
||
| const radios = pageHistoryElement.querySelectorAll('input[name="a"], input[name="b"]'); | ||
|
|
||
| function updateCompareButton() { | ||
| const checkedRadioA = pageHistoryElement.querySelector('input[name="a"]:checked'); | ||
| const checkedRadioB = pageHistoryElement.querySelector('input[name="b"]:checked'); | ||
|
|
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
I prefer the "When Who Comment" information in their own columns for ease of scanning, this change bunches all three into a line wrapped single cell which is going to slow down review. The screenshot shows most of the page taken up by a large, mostly whitespace diff column that has the same buttons repeated, and more repetition by labelling every radio button A or B. Most of the text content on the page is now repetitive labels, and the expense of the history information. Can we keep the existing column layout, which is useful, and make the diff buttons take up less space in order to emphasise the relevant information? Wikipedia uses compact (cur|prev) text links very close to radio buttons for these features which is far much more compact. Other aspects of the Wikipedia history aren't as good, the changes are quite dense; OL's current table layout is cleaner. Also, FWIW, I use the arbitrary compare A-B option all the time because many edits come in batches where the same editor make multiple edits in a row. I'm not sure I've ever used the compare against current as it likely groups too many unrelated changes by different users over time. The main feature as I see it is to add single click diff . Making the comment clickable is one approach, but the link styling with underlines might make the comment more awkward to read. |
|
@hornc Thank you for the detailed feedback. Useful to hear from someone who uses this regularly. I can revert the merged Edit column and keep the When/Who/Comment separate. On the diff buttons, would you be happy with compact text links (cur | prev) close to the existing radio buttons, similar to Wikipedia's layout? That would reduce repetition. @lokesh given the feedback, should I scale back the styling and focus on just adding the quick-link functionality in a compact form? Additionally, what can be done to implement a single click diff, as suggested by @hornc? I think this changes the scope of this fix. (I can also go ahead and implement the persistent A/B selections across pagination while I'm at it) |
- Use const instead of var for compareBtns (ESLint) - Source the 'Compare A with B' link text from a data-i18n payload on #pageHistory so it is translatable, with named placeholders so translators can reorder the revision numbers
- Restructure the table into Rev / When / Who / Comment / Diff / Pick columns - Add arbitrary A/B revision comparison with selected-row highlighting - Mobile: stack rows into cards with a rev # gutter, content column, and a right-hand A/B radio column; uniform 0.85rem text; left-aligned Changes / vs current buttons - Collapse rows with no comment so they reserve no space - Remove the "Current" badge on both desktop and mobile
for more information, see https://pre-commit.ci
|
@hornc Thanks for the thoughtful write-up. 🙏
Collecting the comment, author, and date into a single cell could have been better executed in terms of visual hierarchy, but it will still not beat multiple columns for skimming. Additionally, the information density via whitespace adjustments, shorter labels, etc. can be increased. Your points are all noted.
Wikipedia's history UI is very dense and efficient, but intimidating. Hoping to find something a little more approachable but still focused on the 'expert' user. Updates incoming shortly. Thanks again! |
- Drop the A/B selection accent bar on mobile (the desktop bar lives on the number cell, which is only as tall as the rev number here) - Remove the comment cell top margin and font-size override on mobile - Bump the page-book.css bundlesize budget to 17KB to match the build
Track the A/B revision selection in the URL query string so it survives pagination, and surface it in a sticky comparison bar that works even when the picked revisions live on different pages.
|
Updated video: history.mp4
|
Closes #12628
This PR redesigns the history page comparison UI, introduces history page handling logic and styling changes.
Technical
Testing
Navigate to https://openlibrary.org/help?m=history to verify whether the UI is new and the behaviour is as expected
Screenshot
Stakeholders
@lokesh