fix: admin UI and community cloud fixes - #439
Conversation
| * `beginWorking` can reject re-entry within the same tick, before React | ||
| * re-renders with the disabled buttons. | ||
| */ | ||
| const useWorkingState = () => { |
There was a problem hiding this comment.
This feels incredibly over-engineered – why is this necessary over simply using a single state variable?
There was a problem hiding this comment.
A single state variable cannot reject the second click of a double-click: state updates are asynchronous, so both clicks read isWorking === false before the re-render disables the buttons, and the action fires twice. The ref mirrors the state synchronously so beginWorking() can refuse re-entry within the same tick. Happy to simplify if there is a leaner pattern that covers that case.
|
|
||
| const newUrl = buildUrl(window.CODE_SNIPPETS.urls.edit, { id: response.id }) | ||
| window.history.pushState({}, document.title, newUrl) | ||
| window.history.replaceState({}, document.title, newUrl) |
There was a problem hiding this comment.
Why use replaceState instead of pushState? pushState feels more user-friendly given we are changing the page URL itself.
There was a problem hiding this comment.
With pushState, pressing back after the first save returns to the add-snippet URL, which renders an empty editor for a snippet that now exists — saving again from there creates a duplicate. replaceState lets the edit URL take over the history entry, so back leads to the previous page instead of the stale form. Happy to switch it back if keeping the extra history entry is preferred.
There was a problem hiding this comment.
Ah fair enough – I suppose that makes sense. Just looking for the best user behaviour here.
| </>} | ||
| </span> | ||
| {count && <span className="subnav-count">{count}</span>} | ||
| {count ? <span className="subnav-count">{count}</span> : null} |
There was a problem hiding this comment.
Please do not update this – && is cleaner.
| > | ||
| {`${label} `} | ||
| <span className="count">{ | ||
| // translators: %d: number of snippets in the current view. |
There was a problem hiding this comment.
Pretty sure these comments need to be on the line beforehand.
There was a problem hiding this comment.
Feels unnecessary to create a whole new utility file for one function.
2 participants
Admin UI and community cloud fixes, with matching test coverage.
Changes
extraActionsrender slot for additional footer buttons.utils/snippets/cloud.tshelper, with each view owning a single select-all control. The snippets table accepts optional selection state so it can render without bulk actions.aria-label; empty snippet type counts no longer render a stray zero in the subnav.Verification