0b93790 · reflex-dom · git.iko.soy

git.iko.soy/reflex-dom

More careful handling of value/checked statuses for inputElement

Both the static renderer and hydration builder had issues here.

commit0b9379019c42a66ecfc0f9be2a7596ff0d33deb8 authorTom Smalley <tfsmalley@gmail.com> dateWed 8 Jul 2020, 20:39 (6 years ago) parent6f06fbe

2 files changed+55−16

compare with develop

unifiedsplit

reflex-dom-core/src/Reflex/Dom/Builder/Immediate.hs+35−8 reflex-dom-core/src/Reflex/Dom/Builder/Static.hs+20−8

reflex-dom-core/src/Reflex/Dom/Builder/Immediate.hs+35−8

@@ -11,6 +11,7 @@

1111{-# LANGUAGE GeneralizedNewtypeDeriving #-}

1212{-# LANGUAGE LambdaCase #-}

1313{-# LANGUAGE MultiParamTypeClasses #-}

14+{-# LANGUAGE MultiWayIf #-}

1415{-# LANGUAGE OverloadedStrings #-}

1516{-# LANGUAGE PatternGuards #-}

1617{-# LANGUAGE PolyKinds #-}

@@ -321,7 +322,7 @@ instance (Reflex t, MonadFix m) => DomRenderHook t (HydrationRunnerT t m) where

321322-- done *before* the switchover to immediate mode - this is most likely some

322323-- form of 'hold' which we want to remove after hydration is done

323324{-# INLINABLE addHydrationStepWithSetup #-}

324−addHydrationStepWithSetup :: (Adjustable t m, MonadIO m) => m a -> (a -> HydrationRunnerT t m ()) -> HydrationDomBuilderT s t m ()

325+addHydrationStepWithSetup :: MonadIO m => m a -> (a -> HydrationRunnerT t m ()) -> HydrationDomBuilderT s t m ()

325326addHydrationStepWithSetup setup f = getHydrationMode >>= \case

326327 HydrationMode_Immediate -> pure ()

327328 HydrationMode_Hydrating -> do

@@ -926,14 +927,28 @@ inputElementInternal cfg = getHydrationMode >>= \case

926927 doc <- askDocument

927928 -- Expected initial value from config

928929 let v0 = _inputElementConfig_initialValue cfg

929− addHydrationStep $ do

930+ c0 = _inputElementConfig_initialChecked cfg

931+ valuesAtSwitchover = do

932+ v <- maybe (pure $ pure v0) (hold v0) (_inputElementConfig_setValue cfg)

933+ c <- maybe (pure $ pure c0) (hold c0) (_inputElementConfig_setChecked cfg)

934+ pure (v, c)

935+ addHydrationStepWithSetup valuesAtSwitchover $ \(switchoverValue', switchoverChecked') -> do

936+ switchoverValue <- sample switchoverValue'

937+ switchoverChecked <- sample switchoverChecked'

930938 domElement <- liftIO $ readIORef domElementRef

931939 let domInputElement = uncheckedCastTo DOM.HTMLInputElement domElement

932940 getValue = Input.getValue domInputElement

933− -- The browser might have messed with the value, or the user could have

934− -- altered it before activation, so we set it if it isn't what we expect

935− liftJSM getValue >>= \v0' -> do

936− when (v0' /= v0) $ liftIO $ triggerChangeByUI v0'

941+ -- Fire the appropriate events if the value has changed somehow

942+ liftJSM getValue >>= \realValue -> liftIO $ if

943+ -- The user could have altered the value before switchover

944+ | realValue /= switchoverValue -> triggerChangeByUI realValue

945+ -- When the value has been updated by setValue before switchover, but

946+ -- the user hasn't entered text, we must send an update here to remain

947+ -- in sync. This is because the later requestDomAction based on the

948+ -- setValue event will not capture events happening before postBuild,

949+ -- because this code runs after switchover.

950+ | v0 /= switchoverValue -> triggerChangeBySetValue switchoverValue

951+ | otherwise -> pure ()

937952 -- Watch for user interaction and trigger event accordingly

938953 requestDomAction_ $ (liftJSM getValue >>= liftIO . triggerChangeByUI) <$ Reflex.select (_element_events e) (WrapArg Input)

939954 for_ (_inputElementConfig_setValue cfg) $ \eSetValue ->

@@ -947,7 +962,19 @@ inputElementInternal cfg = getHydrationMode >>= \case

947962 ]

948963 liftIO . triggerFocusChange =<< Node.isSameNode (toNode domElement) . fmap toNode =<< Document.getActiveElement doc

949964 requestDomAction_ $ liftIO . triggerFocusChange <$> focusChange'

950− Input.setChecked domInputElement $ _inputElementConfig_initialChecked cfg

965+ -- Fire the appropriate events if the checked state has changed somehow

966+ liftJSM (Input.getChecked domInputElement) >>= \realChecked -> liftIO $ if

967+ -- The user could have clicked the checkbox before switchover, we only

968+ -- detect cases where they flipped the state

969+ | realChecked /= switchoverChecked -> triggerCheckedChangedByUI realChecked

970+ -- When the checked state has been updated by setChecked before

971+ -- switchover, but the user hasn't changed the state, we must send an

972+ -- update here to remain in sync. This is because the later

973+ -- requestDomAction based on the setChecked event will not capture

974+ -- events happening before postBuild, because this code runs after

975+ -- switchover.

976+ | c0 /= switchoverChecked -> triggerCheckedChangedBySetChecked switchoverChecked

977+ | otherwise -> pure ()

951978 _ <- liftJSM $ domInputElement `on` Events.click $ do

952979 liftIO . triggerCheckedChangedByUI =<< Input.getChecked domInputElement

953980 for_ (_inputElementConfig_setChecked cfg) $ \eNewchecked ->

@@ -960,7 +987,7 @@ inputElementInternal cfg = getHydrationMode >>= \case

960987 let getMyFiles xs = fmap catMaybes . mapM (FileList.item xs) . flip take [0..] . fromIntegral =<< FileList.getLength xs

961988 liftIO . triggerFileChange =<< maybe (return []) getMyFiles mfiles

962989 return ()

963− checked' <- holdDyn (_inputElementConfig_initialChecked cfg) $ leftmost

990+ checked' <- holdDyn c0 $ leftmost

964991 [ checkedChangedBySetChecked

965992 , checkedChangedByUI

966993 ]

reflex-dom-core/src/Reflex/Dom/Builder/Static.hs+20−8

@@ -312,16 +312,28 @@ instance SupportsStaticDomBuilder t m => DomBuilder t (StaticDomBuilderT t m) wh

312312 return (e, result)

313313 {-# INLINABLE inputElement #-}

314314 inputElement cfg = do

315− -- Tweak the config to update the "value" attribute appropriately.

315+ -- Tweak the config to update the "value" and "checked" attributes appropriately.

316316 -- TODO: warn upon overwriting values.

317− let adjustedConfig = (_inputElementConfig_elementConfig cfg)

318− & elementConfig_initialAttributes %~ Map.insert "value" (_inputElementConfig_initialValue cfg)

319− & elementConfig_modifyAttributes %~ (case _inputElementConfig_setValue cfg of

320− Nothing -> id

321− Just e -> \e' -> (Map.singleton "value" . Just <$> e) <> e')

317+ let setInitialValue = Map.insert "value" (_inputElementConfig_initialValue cfg)

318+ setUpdatedValue updatedAttrs = case _inputElementConfig_setValue cfg of

319+ Nothing -> updatedAttrs

320+ Just e -> (Map.singleton "value" . Just <$> e) <> updatedAttrs

321+ setInitialChecked = case _inputElementConfig_initialChecked cfg of

322+ True -> Map.insert "checked" "checked"

323+ False -> id

324+ setUpdatedChecked updatedAttrs = case _inputElementConfig_setChecked cfg of

325+ Nothing -> updatedAttrs

326+ Just e -> (Map.singleton "checked" (Just "checked") <$ e) <> updatedAttrs

327+ adjustedConfig = (_inputElementConfig_elementConfig cfg)

328+ & elementConfig_initialAttributes %~ setInitialValue . setInitialChecked

329+ & elementConfig_modifyAttributes %~ setUpdatedValue . setUpdatedChecked

322330 (e, _result) <- element "input" adjustedConfig $ return ()

323− let v0 = constDyn $ cfg ^. inputElementConfig_initialValue

324− let c0 = constDyn $ cfg ^. inputElementConfig_initialChecked

331+ v0 <- case _inputElementConfig_setValue cfg of

332+ Nothing -> pure $ constDyn (cfg ^. inputElementConfig_initialValue)

333+ Just e -> holdDyn (cfg ^. inputElementConfig_initialValue) e

334+ c0 <- case _inputElementConfig_setChecked cfg of

335+ Nothing -> pure $ constDyn $ _inputElementConfig_initialChecked cfg

336+ Just e -> holdDyn (_inputElementConfig_initialChecked cfg) e

325337 let hasFocus = constDyn False -- TODO should this be coming from initialAtttributes

326338 return $ InputElement

327339 { _inputElement_value = v0