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

Add cache busting to inputs #552

Merged
merged 1 commit into from
May 24, 2024
Merged

Add cache busting to inputs #552

merged 1 commit into from
May 24, 2024

Conversation

JJ-Atkinson
Copy link
Contributor

I believe this cache busting is what's required to make the input work properly. Reference in slack: https://clojurians.slack.com/archives/C68M60S4F/p1716272709162819

Here's some workspace test code:

(defsc InputComponentTest [this props]
  {:query         [:a/id
                   :act-v]
   :initial-state {:a/id  0
                   :act-v 1}
   :ident         (fn [] [:a/id 0])
   :route-segment ["hi"]}
  (dom/div
    {:style {:display        "flex"
             :flex-direction "column"
             :gap            "0.45em"
             :align-items    "flex-start"}}
    "State"
    (dom/code
     (dom/pre
      (pr-str props)))

    (dom/strong "Instructions")
    (dom/ul
    {:style {:paddingLeft "1em"}}
      (dom/li "Click `Set value to 1`")
      (dom/li "Observe the input and :act-v being set to 1")
      (dom/li "Clear the input and type `2`")
      (dom/li "Click `Set the value to 1` and observe that it is indeed 1")
      (dom/li "Click `Set the value to 2` and observe that it NOT 2, it still shows 1 though the state shows `2`"))

    (dom/div
      {:style {:display "flex"
               :gap     "0.5em"}}
      (dom/span "input")
      (f.inputs/ui-int-input
       {:value    (:act-v props)
        :onChange (fn [x] (mut/set-value!! this :act-v x))
        :key      "key"
        :debug    "true"}))

    (dom/button
      {:onClick (fn [x] (mut/set-value! this :act-v 1))}
      "Set value to 1")

    (dom/button
      {:onClick (fn [x] (mut/set-value! this :act-v 2))}
      "Set value to 2")))

The underlying problem is the cachedValue was still being treated as valid even when it is no longer being represented in the model. So, whenever the model is updated externally the cache is "busted" and remains so until the user resumes control of the input by typing in it again.

Let me know if this change causes any issues - debugging dom inputs is troublesome and I'm not 100% confident that this addresses all edge cases.

@awkay
Copy link
Member

awkay commented May 21, 2024

So, the main edge case to test is this:

  1. Set up a controlled input
  2. Use async transactions (e.g. single ! variant with no plugins or special compile options in Fulcro)
  3. Type into the input, move the cursor left (not deleting) and try to put something in the middle of the existing text

The problems you may experience are:

  • The cursor annoying jumps to the end of the input after one character typed
  • The input loses focus on each character

The other possible issue is almost impossible to fix: slow async transactions can cause rapid typing to lose values. The only fix for that is to switch to synchronous processing

@JJ-Atkinson
Copy link
Contributor Author

It's probably possible to accomplish this with uncontrolled inputs and hooks. I'm not quite sure how though. I've gotten stuck with infinate renders 😂. I've done the testing you mentioned with the code in the PR already - see the attached video using our internal bigdec percent input. It's about as good as I could expect. I'll report back if I run into any issues with this code on our app by next tuesday.

(defn simplify-decimal-str 
  "Remove trailing . and treat `\"-\"` or `\"+\"` as empty strings."
  [s]
  (when (and s (< 0 (count s)))
    (case (nth s (dec (count s)))
      ("+" "-" ".") (subs s 0 (dec (count s)))
      s)))

(def ui-percent-input
  (comp/factory
   (dinputs/StringBufferedInput
    ::PercentInput
    {:model->string (fn [v] (dec-math/numeric->str (dec-math/* (dec-math/numeric v) 100M)))
     :string->model (fn [s] (dec-math/div (dec-math/numeric (simplify-decimal-str s)) 100M))})))
Screencast.from.2024-05-21.16-18-42.webm

@awkay awkay merged commit b2383f1 into fulcrologic:main May 24, 2024
1 check passed
@awkay
Copy link
Member

awkay commented May 24, 2024

Looks good. I can't get it to break either.

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

Successfully merging this pull request may close these issues.

2 participants