From 50d5d62725adff56f75d7342ce19abc5a5ba6134 Mon Sep 17 00:00:00 2001 From: Hardy Jones <47793030+joneshf-cn@users.noreply.github.com> Date: Wed, 28 Apr 2021 10:31:00 -0700 Subject: [PATCH 1/2] FBCM-4170 Don't raise from a query It seems like a query causing a raise can initiate an infinite loop. When we call `setSelection` everywhere, we always get the same behavior. That's good for having uniform behavior. Unfortunately the way this was setup seemed to cause an infinite loop sometimes. If a parent told either `Ocelot.Components.DatePicker.Component.SetSelection _` or `Ocelot.Components.TimePicker.Component.SetSelection _`, they would end up raising `Ocelot.Components.DatePicker.Component.SelectionChange _`, or `Ocelot.Components.TimePicker.Component.SelectionChange _`, respectively. Depending on how the parent handled that, it could end up telling the same query and kicking off an infinite loop. We try to address that by not raising the query we just responded to only in the case where we just received the query. The other cases still should still raise their output as before since they aren't initiated from the query. Cc: Gabe Johnson --- src/Components/DatePicker/Component.purs | 9 ++++++++- src/Components/TimePicker/Component.purs | 7 ++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/Components/DatePicker/Component.purs b/src/Components/DatePicker/Component.purs index 311759b6..4b2c7de8 100644 --- a/src/Components/DatePicker/Component.purs +++ b/src/Components/DatePicker/Component.purs @@ -241,6 +241,13 @@ setSelection selection = do H.raise $ SelectionChanged selection synchronize +setSelectionWithoutRaising :: forall m. MonadAff m => Maybe Date -> CompositeComponentM m Unit +setSelectionWithoutRaising selection = do + st <- H.get + let targetDate = maybe st.targetDate (\d -> (year d) /\ (month d)) selection + H.modify_ _ { selection = selection, targetDate = targetDate } + synchronize + -------------------------- -- Embedded > handleAction @@ -318,7 +325,7 @@ embeddedHandleQuery = case _ of SetDisabled disabled a -> Just a <$ do H.modify_ _ { disabled = disabled } SetSelection selection a -> Just a <$ do - setSelection selection + setSelectionWithoutRaising selection --------------------------- -- Embedded > handleMessage diff --git a/src/Components/TimePicker/Component.purs b/src/Components/TimePicker/Component.purs index 79389e08..83d12e8e 100644 --- a/src/Components/TimePicker/Component.purs +++ b/src/Components/TimePicker/Component.purs @@ -234,7 +234,7 @@ embeddedHandleQuery = case _ of H.modify_ _ { disabled = disabled } SetSelection selection a -> Just a <$ do - setSelection selection + setSelectionWithoutRaising selection embeddedRender :: forall m. CompositeComponentRender m embeddedRender s = @@ -354,3 +354,8 @@ setSelection selection = do H.modify_ _ { selection = selection } H.raise $ SelectionChanged selection synchronize + +setSelectionWithoutRaising :: forall m. Maybe Time -> CompositeComponentM m Unit +setSelectionWithoutRaising selection = do + H.modify_ _ { selection = selection } + synchronize From c966916230bb4f96b990ec7d5eb91305284cc0f5 Mon Sep 17 00:00:00 2001 From: David Date: Wed, 28 Apr 2021 13:38:58 -0700 Subject: [PATCH 2/2] FBCM-4170 Consolidate `setSelection` logic We don't want to have to maintain two different code paths for doing mostly the same thing. We want to avoid a developer changing one in the future without changing the other. We update `setSelection` to defer to the logic in `setSelectionWithoutRaising` before raising. --- src/Components/DatePicker/Component.purs | 5 +---- src/Components/TimePicker/Component.purs | 3 +-- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/Components/DatePicker/Component.purs b/src/Components/DatePicker/Component.purs index 4b2c7de8..2a3bebbf 100644 --- a/src/Components/DatePicker/Component.purs +++ b/src/Components/DatePicker/Component.purs @@ -235,11 +235,8 @@ synchronize = do setSelection :: forall m. MonadAff m => Maybe Date -> CompositeComponentM m Unit setSelection selection = do - st <- H.get - let targetDate = maybe st.targetDate (\d -> (year d) /\ (month d)) selection - H.modify_ _ { selection = selection, targetDate = targetDate } + setSelectionWithoutRaising selection H.raise $ SelectionChanged selection - synchronize setSelectionWithoutRaising :: forall m. MonadAff m => Maybe Date -> CompositeComponentM m Unit setSelectionWithoutRaising selection = do diff --git a/src/Components/TimePicker/Component.purs b/src/Components/TimePicker/Component.purs index 83d12e8e..519293bd 100644 --- a/src/Components/TimePicker/Component.purs +++ b/src/Components/TimePicker/Component.purs @@ -351,9 +351,8 @@ synchronize = do setSelection :: forall m. Maybe Time -> CompositeComponentM m Unit setSelection selection = do - H.modify_ _ { selection = selection } + setSelectionWithoutRaising selection H.raise $ SelectionChanged selection - synchronize setSelectionWithoutRaising :: forall m. Maybe Time -> CompositeComponentM m Unit setSelectionWithoutRaising selection = do