Repository navigation
Peer review pages and the Reviews tab on house components (#2658) - #2706
Merged
Merged
Conversation
The reviewer and contributor pages lose their duplicated <style> blocks, and the Summary tab the stylesheet its script injected: the styles move into theming/scss/layouts/_review.scss on tokens, and the Summary tab's states render as house badges. Every action carries btn + a role on itself; the container-selector rules in _review.scss and _profile.scss are deleted. The field saves are primary (no more yellow/red toggle, updateSubmitButtonColor is gone). The arrow buttons get names. Reviews tab (login:reviews): Delete asks the confirm dialog and posts from the tab's own small script with the page's CSRF token, instead of loading the whole reviewer module, which never bound it (#2674). "All reviews" lists again: ReviewsView passed *_organizationed keys since the groups -> organizations rename while the template read *_grouped. Statuses are badges, empty lists the empty state, the placeholder history menus are gone. The peer review pages join RESTYLED_PAGES with a review in the fixture; the status-colour scans now cover all five colours. Closes #2674 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- The field saves are secondary: Submit (or Finish, which replaces it while Submit is disabled) is the page's one primary action. - The Reviews tab's Delete toasts a network failure too, not only a refused request. - The Summary's suggested value keeps a warning-tinted colour, mixed from tokens as the badge does. - The info popover uses Bootstrap 5's data-bs-trigger. - The Reviews tab's tests live in login/tests/test_reviews_tab.py; the CSRF check reads the response's cookie, not the script's text; the duplicate opening-tag helper is element_with_id; summary.js drops an import its deleted function was the only user of. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…review-pages # Conflicts: # versions/changelogs/current.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2658. Closes #2674.
Part of the stage-2 design spec #2632. No migration, no deploy step.
What changes
Reviewer and contributor pages (
dataedit:peer_review_reviewer/_create/_contributor)<style>blocks are gone, and so is the stylesheetsummary.jsinjected into the head. All three were copies of one sheet in BS3/Material colours. The rules now live intheming/scss/layouts/_review.scssand read only tokens:--bs-success/--bs-warning/--bs-dangerfor the review states, plus--oep-text-muted,--oep-border-strong,--oep-surfaceand--oep-surface-hover.btnand a role on itself. The container rules in_review.scss(.review__btn button,.review__description button,.review__btn-nav button,.review-bar__action--* button) are deleted.btn-warningand the script flipped them between yellow and red with the chosen state.updateSubmitButtonColor()is deleted, so the saves keep one role.aria-label, invisible text. The phase-2 notice uses the alert include's markup (role="status", no inline width).viewBox+cx/cy/r), so one rule draws both. Before, the contributor page setr/cx/cyin its own CSS.alert()s were already toasts after One confirm dialog and one toast region replace every native dialog #2643. Neither needed work here.Reviews tab (
login:reviews)data-confirm*, danger) and posts from a small inline module on the tab. The tab no longer loadsopr_reviewer.js, which never bound the button and logged five errors here.sendJsonand got 403 "CSRF cookie not set" in Chrome.latest_active), notlatest_review_id, which was the last review of any status. That context variable is removed.ReviewsViewpassed*_organizationedkeys while the template read*_grouped. The view's keys are restored. The contributor waiting line readcontributor_reviewed_active.latest.table, which does not exist, and now readscontributor_reviewed.latest_active.table.empty_statecomponent (compact).btn-secondary btn-sm. Its placeholder menus ("Action / Another action") are deleted rather than restyled as house dropdowns: the trigger is disabled, so they could never open. The spec names "peer-review history" among the dropdowns, which is the one departure from a spec line here.aria-label, and uses Bootstrap 5'sdata-bs-trigger._profile.scssloses the pill rule and the.profile-rvw__item-history button { btn-light dropdown-toggle }rule.Lint and scans
RESTYLED_PAGES, and the comment line that kept them off is removed.PageFixturenow carries an open review by the fixture user, with the table's metadata as a real review holds it. Without that metadata the reviewer route 500s, the trap Underline tabs and house dropdowns #2648 hit. This is also what gives the Reviews tab, already on the list, buttons to lint at all; before, it passed with nothing on it.btn-warningand the last@extend .btn-light.Tests
dataedit/tests/test_peer_review_pages.py(4):<style>in either template;login/tests/test_reviews_tab.py(5). It used to assert Fix six develop defects found by the design census #2634's "the reviewer script is a module" and now asserts the tab loads none of the review page's scripts. Also covered:base+login+dataedit: 1,361 tests. The first run found two failures. One was the Fix six develop defects found by the design census #2634 test above, rewritten. The other: the new fixture'sself.reviewshadowedMovedPagesTest's own empty review and changed a develop-captured snapshot. The fixture's attribute is nowpeer_review, and the snapshot is unchanged. The touched modules are green after the review fixes (test_page_lint,test_page_header_moved,test_reviews_tab,test_peer_review_delete,test_peer_review_pages,test_tabs_and_dropdowns,test_confirm_dialog). I did not run the whole suite again after the last commit.dataedit/static/peer_reviewsuites pass (12 tests). They ran under a scratch vitest 3 on Node 20, because the repo's vitest needs Node 22. The happy-dom suites fail to load under that runner on develop's checkout too, so that failure is the runner.Checks
benchmarks/homepage/compare.mjs: 8/8 byte-identical, whole page, nothing masked, before the branch and after the last theme rebuild.check_catalogue: complete, 19 entries.theming/literal_scan.mjs: 0.sass@1.77.0+bootstrap@5.2.0. Develop's sources rebuild develop'sbootstrap.min.cssbyte for byte, andstock_tokens.cssis unchanged.Left alone, for the maintainer
.reviewgrid is 50/50 at every width (measured on develop by stashing the branch). That is layout work, not this ticket's.aria-pressed, the same as on develop. They are a choice; the segmented control (btn-check+btn-outline-primary) would fit if that is wanted..opr-history__item--*, the Summary dots, andSTATUS_META. The "NEW" mark repeats the badge's warning mix instead of extending it, because it is a pseudo-element..reviewer-itemis dead and is Delete the legacy theme layer and dead code #2660's.Took develop after #2704 (
33bc40f25)bootstrap.min.cssmerged cleanly, and the merged sources rebuild it byte for byte.baseplus the touched login/dataedit modules pass (276 tests).🤖 Generated with Claude Code