Message flux store to react context - #5967
Merged
Merged
Conversation
Replaces the Fluxible-backed MessageStore/MessageActions with a self-contained MessageProvider (app/hooks/MessageContext.js), following the same conversion pattern used previously for FavouriteStore. Unlike favourites, no non-React module needs message state, and MessageBar renders null until client-mount (no SSR dependency), so the whole thing fits in a single context/hook file with no separate app/data singleton needed: - MessageProvider holds messages/duplicateMessageCounter in a reducer, exposes useMessages/useDuplicateMessageCounter/useMessageActions. - Loads static + remotely fetched config messages once on mount, replacing client.js's former pre-render call to MessageStore.addConfigMessages(config). - MessageBar.js converted from a class component (connectToStores, context.executeAction, context.intl/config) to a function component using useIntl()/useConfigContext() and the new message hooks. - NaviContainer.js's connectToStores(['MessageStore'], ...) wrapper removed; its 'messages' prop was unused dead code. - FavouriteContext.js/FavouriteStopContainer.js now call useMessageActions().addMessage(...) directly instead of context.executeAction(addMessage, ...), removing their Fluxible context dependency for this purpose. - app.js no longer registers MessageStore; app/store/MessageStore.js and app/action/MessageActions.js removed. - test/unit/store/MessageStore.test.js replaced by test/unit/hooks/MessageContext.test.js. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
isIeOrOldVersion()/isIe (app/util/browser.js) fed a staticIEMessage 'please upgrade your browser' banner (config.default.js) shown via processStaticMessages when an old browser was detected. This is unreachable in practice: browserslist already excludes ie<=11, chrome<55 and safari<11 from Babel's build targets, so those browsers fail to parse the JS bundle before React ever mounts - the banner could never actually render for the browsers it targeted. It also had no test coverage. Removing it also retires the sessionStorage-based read-tracking (app/store/sessionStorage.js + its test) added solely to let the 'IE' message (hardcoded id '3') reappear each session instead of being permanently dismissed like normal messages - nothing else used it. isFirefox in browser.js is now also unused and removed; isEdge/isChrome are kept since isSafari's detection still depends on them. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The class->function conversion of MessageBar lost two re-render
guarantees the old Fluxible version relied on:
- MessageStore.markMessageAsRead() always called this.emitChange(),
even for ids (like service alerts) it never stored itself, forcing
every connectToStores-wrapped consumer to re-render.
- The class's this.setState({ slideIndex: ... }) always re-rendered,
even when the value didn't change.
MessageContext's MARK_READ reducer only produces a new state (and
thus a re-render) when the id exists in its own messages Map, which
service alerts never do (they live in MessageBar's local
serviceAlerts state, filtered against getReadMessageIds() at render
time). Combined with useState's bailout on an unchanged value
(closing the only/last message sets slideIndex 0 -> 0), closing a
single service-alert banner updated localStorage but never
re-rendered the component, so the banner stayed visible until the
next full page load.
Track dismissed ids in local state (readMessageIds) and update it in
handleClose so a re-render is guaranteed regardless of slideIndex or
context message changes.
Added a regression test in MessageBar.test.js that clicks the close
button and asserts the banner disappears without a remount (verified
it fails against the pre-fix code).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
vesameskanen
marked this pull request as draft
September 11, 2026 12:02
The MessageStore->MessageContext refactor (caceace) dropped NaviContainer's connectToStores(['MessageStore']) wrapper as "unused dead code" since the messages prop itself was never consumed. However, the subscription's real purpose was to force NaviContainer to re-render when message bar content changes, keeping layout-dependent values like containerTopPosition (derived from mapLayerRef's bounding rect) in sync immediately, per a7fc919. Restore this behavior using the new useMessages() hook. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The message-bar-content/message-bar-container divs and the close-button wrapper div carried no styling of their own (all matching SCSS rules are plain descendant selectors, not child combinators), so they were flattened/removed without any visual change. Verified against navigation.scss and MessageBar.test.js (10/10 passing). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
# Conflicts: # app/hooks/FavouriteContext.jsx # app/store/MessageStore.js
…iContainer MessageContext's MARK_READ reducer previously bailed out with the same state reference when the target id was not tracked in state.messages. Live service alerts (MessageBar) and geolocation messages (PositionActions) are never added via addMessage, so marking them read never changed the context value identity and useMessages() subscribers such as NaviContainer did not re-render. Also, PositionActions.updateGeolocationMessage still dispatched legacy Fluxible actions (AddMessage/MarkMessageAsRead) that have had no listener since MessageStore was removed, so geolocation messages never reached MessageContext at all. Added a messageActions bridge so plain (non-React) modules can call addMessage/markMessageAsRead on the current MessageProvider. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Reading mapLayerRef.getBoundingClientRect() directly during render returned a stale value: React runs all render functions of components re-rendering in the same batch (NaviContainer and MessageBar, both subscribed to MessageContext) before committing any of their DOM changes, so the read reflected the previous layout. This caused the navi cards to only move up on the next unrelated re-render (5-10s later), and stale/overlapping layout when reopening the navigator. containerTopPosition is now state, recomputed in a useLayoutEffect that runs after the DOM commit but before paint, so it always reflects the current message bar height with no visible flicker or overlap. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tring Ported from PR #5974 (app/store/MessageStore.js), which fixed the same bug in the now-removed Fluxible MessageStore. addConfigMessages's guard checked staticMessagesUrl !== undefined, so an empty string (e.g. the Dockerfile's default STATIC_MESSAGE_URL='') still passed and triggered fetch(''), which resolves to the current page and fails to parse the resulting HTML as JSON. Changed to a truthiness check, with a regression test asserting no fetch happens when staticMessagesUrl is ''. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
vesameskanen
marked this pull request as ready for review
September 14, 2026 12:32
# Conflicts: # app/action/PositionActions.js # app/client/app.js # app/client/client.jsx # app/component/FavouriteStopContainer.jsx # app/component/MessageBar.jsx # app/component/itinerary/navigator/NaviContainer.jsx # app/hooks/FavouriteContext.jsx # app/store/MessageStore.js
sharhio
requested changes
Sep 15, 2026
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
2 participants
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.
Summary
Converts the legacy Fluxible
MessageStore(message bar / notifications) to a Reactcontext (
MessageContext), and fixes several bugs uncovered along the way in howmessage-bar state interacts with re-rendering and layout.
Changes
Fluxible
MessageStorewithapp/hooks/MessageContext.jsx, exposingMessageProvider/useMessages()/useMessageActions()for adding and dismissingmessages.
code cleanup no longer relevant after the store conversion.
dismissed via the new context are removed immediately instead of only after a
full page reload.
fix: undefined default prop — follow-up fixes to keep
NaviContainersubscribed correctly to message state.
MessageBar.jsxmarkup (removed redundant nested<div>s, merged two adjacentwrapper divs), verified against SCSS selectors and existing tests.
NaviContainer —
MessageContext'sMARK_READreducer previously bailed out with the samestate reference when the target id wasn't tracked in
state.messages. Liveservice alerts (
MessageBar) and geolocation messages (PositionActions)are never added via
addMessage, so marking them read never changed thecontext value identity, and
useMessages()subscribers (e.g.NaviContainer) never re-rendered.PositionActions.updateGeolocationMessagestill dispatched legacy Fluxibleactions (
AddMessage/MarkMessageAsRead) that have had no listener sinceMessageStorewas removed — geolocation messages never reachedMessageContextat all. Added amessageActionsbridge so plain(non-React) modules can call
addMessage/markMessageAsReadon thecurrent
MessageProvider.containerTopPositionwas read frommapLayerRef.getBoundingClientRect()directly during render, which returned a stale value because React runs all
render functions of components re-rendering in the same batch before
committing DOM changes. This caused the navi cards to only move up on the
next unrelated re-render (5-10s later), and stale/overlapping layout when
reopening the navigator. Now measured in a
useLayoutEffect, which runsafter DOM commit but before paint.
Testing
yarn eslint app/— 0 errors.yarn test-unit— 1153 passing, 1 pending, 0 failing.closing service alerts and geolocation messages, and reopening the navigator
view — no more delayed layout shift or header/message overlap.