Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question on signal updates: formless "child" signals lose state, others don't #25

Open
bbarker opened this issue Aug 1, 2019 · 18 comments
Assignees

Comments

@bbarker
Copy link
Contributor

bbarker commented Aug 1, 2019

Sorry for the barrage of signal questions/issues lately, but I'd say this is my most pressing concern so far.

I have the following outer signal and helper function:

injectLocationFields ::
  Maybe M.InstitutionID ->
  Maybe String ->
  Maybe M.InstitutionType ->
  Maybe String ->
  Maybe M.InstitutionContact ->
  Maybe M.InstitutionSustainability ->
  Maybe (NonEmptyArray M.InstitutionPolicy) ->
  Boolean ->
  Maybe M.Location
injectLocationFields
  (Just institutionID)
  (Just institutionName)
  (Just institutionType)
  superOrganizationName
  (Just institutionContact)
  (Just institutionSustainability)
  (Just institutionPolicies)
  versioning = pure $ {
    institutionID: institutionID
  , institutionName: institutionName
  , institutionType: institutionType
  , superOrganizationName: superOrganizationName
  , institutionContact: institutionContact
  , institutionSustainability: institutionSustainability
  , institutionPolicies: institutionPolicies
  , versioning: versioning
  }
injectLocationFields _ _ _ _ _ _ _ _ = Nothing

accumulateLocation :: Maybe M.Location -> Signal HTML (Maybe M.Location)
accumulateLocation locMay = labelSig' D.h1' "Location" do
  identMay <- accumulateIdent "Identifier"
  instNameMay <- textInput D.span' "Institution Name: "
  display $ D.div' [D.text $ "Testing: " <> (show instNameMay)] -- FIXME: DEBUG
  instTypeMay <- labelSig' D.h3' "Institution Type" $ menuSignal Nothing
  display D.br'
  sOrgMay <- textInput D.span' "Super Organization (optional): "
  icMay <- contactSignalInit
  display $ D.div' [D.text $ "Contact" <> (show icMay)]  -- FIXME: DEBUG
  missionUrlMay <- urlInput D.span' "Mission Statement URL: "
  fundingUrlMay <- urlInput D.span' "Funding Statement URL: "
  sustainMay <- pure $ injectSustainFields missionUrlMay fundingUrlMay
  polsMay <- MF.policySigArray Nothing
  versioning <- labelSig' D.span' "versioning? " $ checkBoxS false
  newLocMay <- pure $ injectLocationFields
    identMay
    instNameMay
    instTypeMay
    sOrgMay
    icMay
    sustainMay
    polsMay
    versioning
  display $ locWidg
  pure newLocMay
  where
    contactSignalInit = MF.contactSignal Nothing
    locWidg :: forall a. Widget HTML a
    locWidg = D.div' [
      D.h3' [D.text "Last submitted location summary for this product:"]
    , D.br'
    , foldMap (\loc -> fold $ MV.spacify $ MV.locElems loc) locMay
    ]

The formless signals in this are contactSignal and policySigArray; contactSignal is simpler and is defined like this:

contactSignal :: Maybe M.InstitutionContact
  -> Signal HTML (Maybe M.InstitutionContact)
contactSignal instContactMay = labelSig' D.h2' "Institution Contact" $
  step instContactMay do
    inputs <- pure $ F.wrapInputFields $ outToInRec instContactMay
    instContact <- D.div' [
      contactForm (initFormState inputs validators)
    , foldMap contactWidg instContactMay
    ]
    pure $ contactSignal $ Just instContact

However, whenever I enter text into a non-formless signal, contactSignal resets and becomes Nothing. All other signals (aside from other formless signals) maintain their values.

An example of a signal that doesn't cause this issue is the textInput and derivative urlInput signals, also called from accumulateLocation:

textInput :: D.El' -> String -> Signal HTML (Maybe String)
textInput tag label = labelSig' tag label $ sig Nothing
  where
    sig :: Maybe String -> Signal HTML (Maybe String)
    sig txtMay = step txtMay do
      newTxt <- D.input [P.unsafeTargetValue <$> P.onChange]
      pure $ sig $ if newTxt == "" then Nothing else Just newTxt

The architecture of these signals seems to be much the same, except under the hood, contactSignal is using a concur-formless widget. Of course, Imay be missing something else important.

Here is the complete code at this commit.

@bbarker
Copy link
Contributor Author

bbarker commented Aug 1, 2019

I do have a plan B (not to use formless - it seems I may not really need it for this app), but would be good to understand this. I'll try to upload some photos after I fix another minor display bug shortly.

@bbarker
Copy link
Contributor Author

bbarker commented Aug 1, 2019

As we can see here, both Record Identifier and Institution Name (using textInput) can have values without trampling each other:

image

I can also add data to the Institution Contact form and save it, which causes a new value to be emitted from contactSignal, which is shown below the form. Note that it doesn't trample the other values:

image

But as soon as I add a character (an _ in this case) to one of the textInput fields (Record Identifier in this case), the Institution Contact data is reset to nothing:

image

@bbarker
Copy link
Contributor Author

bbarker commented Aug 1, 2019

Forgot to mention the branch and commit for the above code.

@bbarker
Copy link
Contributor Author

bbarker commented Aug 8, 2019

Any thoughts on areas to explore for this issue? I'm using concur-formless and not formless-independent, though my current guess is that wouldn't matter much.

@ajnsit ajnsit self-assigned this Aug 19, 2019
@ajnsit
Copy link
Member

ajnsit commented Aug 19, 2019

Hi, sorry for the delayed response, I was away on a holiday. I will take a look and get back to you.

@bbarker
Copy link
Contributor Author

bbarker commented Aug 20, 2019

Thanks @ajnsit - sorry for bugging you while on vacation, hope it was a good one - and happy (belated) independence day!

@ajnsit
Copy link
Member

ajnsit commented Aug 21, 2019

@bbarker Do you have instructions on building the metajelo-ui code that don't use docker?

@bbarker
Copy link
Contributor Author

bbarker commented Aug 21, 2019

@ajnsit I just added them to the README (make sure you are looking at the editable_arrays branch); I hope it is fairly standard, let me know if you encounter any build issues. Basically, the Docker container is just supplying some of the standard build tools, nothing unusual as far as I recall.

Also, I took the opportunity to move the 'app' folder contents to the repo root and update to purescript 0.13.3

@bbarker
Copy link
Contributor Author

bbarker commented Aug 21, 2019

Ah, i also added npm run debug, not mentioned in the readme, for making unminified builds.

@bbarker
Copy link
Contributor Author

bbarker commented Aug 26, 2019

@ajnsit reading over #28 made me wonder if this could be an issue with how formless and react interact - does formless somehow need to keep track of its internal state and provide that as a defaultValue, behind the scenes to React (at least in the Concur implementations of formless), assuming the entire form is being recreated. Otherwise, maybe value just needs to be set accordingly, so that state is preserved when other parts of the Concur (React) app cause a render to occur?

I admit I'm not familiar enough with formless at the moment to really know where to begin looking; I'm working of purescript-concur-formless but maybe I should switch to formless-independent? I still see some Halogen imports, for instance.

@ajnsit
Copy link
Member

ajnsit commented Aug 26, 2019

Hey @bbarker apologies for the late response. I finally got around to building your code and checking it out.

I think you are right about it being the same problem as #28. The input will not retain its value unless you either explicitly handle it in Concur, OR use defaultValue instead of value. So for example, the following code seems to work for me -

contactForm :: FState -> Widget HTML M.InstitutionContact
...
        P.defaultValue $ F.getInput proxies.email1 fstate.form
...

@ajnsit
Copy link
Member

ajnsit commented Aug 26, 2019

BTW, the reason why it works for textInput is because you don't use either value or defaultValue. That is also fine, but only if you never want to programmatically control the value of the input.

@bbarker
Copy link
Contributor Author

bbarker commented Aug 27, 2019

@ajnsit excellent! I tried the defaultValue alternative and it seemed to work for the contact email. However, in menu, used for the "Contact type", it seems to already be using defaultValue in the appropriate location.

I also tried playing around with P.selected to see if perhaps that was the right way, but the issue still exists:

menu ::  opt s form e o restF restI inputs fields
   . IsSymbol s
   => IsOption opt
   => BoundedEnum opt
   => Newtype (form Record F.FormField) (Record fields)
   => Cons s (F.FormField e opt o) restF fields
   => Newtype (form Variant F.InputFunction) (Variant inputs)
   => Cons s (F.InputFunction e opt o) restI inputs
   => form Record F.FormField
  -> SProxy s
  -> Widget HTML (F.Query form)
menu form field = D.select
  [ P.defaultValue $ toOptionValue $ currentOpt form
  , (F.set field <<< fromOptionValue <<< P.unsafeTargetValue) <$> P.onChange
  ]
  (upFromIncluding (bottom :: opt) <#> \opt ->
    D.option [
      P.value (toOptionValue opt)
    , P.selected $ isSelected opt (Just $ currentOpt form)
    ] [D.text (toOptionLabel opt)])
  where
    currentOpt form = F.getInput field form

isSelected ::  opt. BoundedEnum opt => opt -> Maybe opt -> Boolean
isSelected opt (Just selOpt) = opt == selOpt
isSelected _ _ = false

@ajnsit
Copy link
Member

ajnsit commented Aug 27, 2019

The contact type field seems to behave fine for me (i.e. does not get reset). Can you tell me the exact steps you are following?

@bbarker
Copy link
Contributor Author

bbarker commented Aug 27, 2019

@ajnsit sure:

  1. choose the only other option (dataCustodian) from Contact type
  2. Type anything in the first field (Record Identifier)
  3. Notice that Contact type resets to "(None)"

@bbarker
Copy link
Contributor Author

bbarker commented Aug 27, 2019

@ajnsit I see what you mean though - this is hard to reproduce! Without recompiling, I've been able to get both positive and negative results. Some observations (in addition to the above steps):

  1. Having an invalid email address entered increases the likelihood the menu select is reset.
  2. Having no email address seems to increase the likelihood it won't be reset.
  3. If the email address is empty, and the you select dataCustodian, then go back to type in the email address field, it will reset the dataCustodian selection to None.

@bbarker
Copy link
Contributor Author

bbarker commented Oct 10, 2019

If it helps, there's now a live CI/CD demo at https://labordynamicsinstitute.github.io/metajelo-ui/

@bbarker
Copy link
Contributor Author

bbarker commented Mar 6, 2020

Just noting (mostly to myself) that this still is an issue on recent builds of metajelo-ui.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants