refactor(event-form): move the CFP reopen section into the Materials panel - #1047
refactor(event-form): move the CFP reopen section into the Materials panel#1047caseylocker wants to merge 3 commits into
Conversation
…panel The reopen control sat at the top of the activity form, above Submitter, where it was visible but not grouped with the thing it governs. Moved it inside the Materials panel, below the materials table. The panels are a single open accordion (showSection starts at "main"), so the panel is collapsed on load and the move would otherwise hide whether a talk has a live window. The panel title carries that state instead: Materials Materials — submission reopened until August 22, 2026 6:51 am Title and section body share one gate, isReopenSectionVisible(). Keying the title on isSubmissionReopened() alone would announce a deadline in exactly the case the comment on isReopenApplicable warns about: a live grant whose plan window was since extended, which the server no longer honours. There is already a test for that case and it now covers the title too. Also: * Panel gets an explicit id="materials". uicore's Panel derives its DOM id from the title when none is passed, so a dynamic title would otherwise move the id every time a grant opens or closes. * Drops the "CFP submission window" label inside the section. The panel title already names the group, and two headings stacked read as noise. * renderEventForm in the test file expands the panel before asserting, since uicore's Panel mounts children only while expanded. queryByText, not getByText, so the tests that assert the control is absent still exercise a real absence rather than passing on a collapsed panel. * Test fixture gains materials: [], which the Table now actually maps over. Verified in the browser against api.dev.fnopen.com on summit 13 / activity 3535: both title states, the collapsed header, and the explicit panel id. Co-Authored-By: Claude <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR refactors the Activity/Event form UI to move the CFP “reopen submission” controls into the Materials panel, and mirrors reopen-grant state in the Materials panel header so operators can still see the reopen status when the panel is collapsed.
Changes:
- Adds a dynamic Materials panel title (
materials_reopened) when a reopen grant is active and the reopen section is applicable. - Moves the reopen submission controls from the main form body into the Materials panel and pins the panel DOM id to a stable value (
id="materials"). - Updates form tests/fixtures to expand the Materials panel when needed and to cover the new title behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/i18n/en.json | Adds materials_reopened translation key; removes unused section label key. |
| src/components/forms/event-form.js | Introduces shared visibility gate for reopen UI and moves the reopen controls into the Materials panel with a dynamic title and stable id. |
| src/components/forms/tests/event-form.test.js | Expands Materials panel in test helper, adds fixture data, and updates assertions for the moved UI/title. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Copilot review on #1047. The test named "announces the deadline on the collapsed Materials panel title" never collapsed the panel, because renderEventForm expands Materials for every test in the file. That is the one behaviour the move exists to preserve, and it had no coverage. Gating the title on showSection, which would blank the header exactly when an operator needs it, passed all 29 tests. Collapse again after render, and assert the panel body is unmounted so the test cannot drift back to asserting the expanded state. Verified: it passes against current code and fails against that mutation. Co-Authored-By: Claude <noreply@anthropic.com>
Reviewer notes on #1047. Extract the Materials panel title into getMaterialsPanelTitle(), alongside the other reopen getters. Pulls a nested ternary out of a 2500-line render and matches the class's existing getReopenDeadline/getMaxReopenHours shape. The gate it shares with the section body is now held by the collapsed-title test rather than by proximity: mutation-checked by gating the method on showSection, which fails that test and nothing else. Trim four comment blocks. event-form.js on master carries no multi-line comment blocks in 2460 lines, and all four were new here. Kept the two non-obvious claims: entity-from-state in isReopenApplicable, and queryByText-not-getByText in the test helper. 169 suites / 1510 tests pass. eslint: 0 errors, same 327 pre-existing warnings as master. Co-Authored-By: Claude <noreply@anthropic.com>
ref: https://app.clickup.com/t/86bbhwf58
What
The CFP reopen control sat at the top of the activity form, just above Submitter. Visible, but not grouped with the thing it governs. It now lives inside the Materials panel, below the materials table.
The one thing that changes shape
The panels are a single open accordion:
showSectioninitialises to"main"andtoggleSectioncollapses whatever was open. So Materials is closed on page load, and moving the control there costs the ability to see at a glance whether a talk has a live window.The panel title carries that state instead, so nothing operational is lost:
MaterialsMaterials — submission reopened until August 22, 2026 6:51 amWhy the title shares the section's gate
isReopenSectionVisible()is new, and both the title and the section body use it.Keying the title on
isSubmissionReopened()alone would announce a deadline in exactly the case the existing comment onisReopenApplicable()warns about: a grant is issued, then the plan'ssubmission_end_dateis extended past it, so the speaker edits under normal open window rules and the grant is no longer what constrains them. There is already a test for that case (the one smarcet raised on the CFP PR) and it now asserts the title too.Mutation checked: reverting the title to
isSubmissionReopened()alone fails that test and nothing else.Also in here
id="materials"on the Panel. uicore'sPanelderives its DOM id from the title when none is passed, so a dynamic title would otherwise move the id every time a grant opens or closes.reopen_submission_sectionlabel ("CFP submission window") inside the section is gone, along with its i18n key. The panel title already names the group and two stacked headings read as noise.renderEventFormin the test file expands the panel before asserting, because uicore'sPanelmounts children only while expanded. It usesqueryByTextrather thangetByTextso the tests that assert the control is absent still exercise a real absence instead of passing on a collapsed panel.materials: [], which the Table now actually maps over.Verification
api.dev.fnopen.com, summit 13 / activity 3535: both title states, the collapsed header, the expanded section in both grant states, and the explicit panel id confirmed on the live DOM. The grant opened for the screenshots was closed again and verified null on the server.The docs side
The operator guide in the docs vault already follows the control into the Materials panel: the prose in steps 2 and 5, four reshot Show Admin screenshots, and two new ones for the collapsed heading and the close confirm. The speaker portal screenshots are untouched, since that is a different app this does not affect.
That is vault PR #121. It should not merge before this one, because until this ships it documents UI that is not deployed.