A minimalistic ClojureScript interface to React.js
Alex 2020-12-04T03:10:15.337500Z

Hi there! I have the following snippet of code which is currently triggering a bunch of re-renders. I suspect it's due to the combination of doall and the #(set-raudio-ref file-name %) on the audio element. However, when I don't force eval via the doall , the derefing of @audio-elems doesn't get triggered. Any suggestions on how to better structure this?

         (for [{:keys [blob file]} @(rf/subscribe [::views.subs/audio-files])
               :let [file-name (gobj/get file "name")]]
           [:div.flex {:style {:justify-content "space-between"}}
            [:audio {:id file-name
                     :src blob
                     :ref #(set-audio-ref file-name %)}]

             [:div file-name]
             [:div (audio-elem->duration (get @audio-elems file-name))]
             [:button "Send"]
             [:button "Move Back"]]]))

p-himik 2020-12-04T05:25:09.337900Z

It's not because of doall. It likely won't solve your issue but I would write the above code with into and a map transducer to avoid creating an unneeded lazy seq and to, arguably, make the code more obvious. The main reason for re-renders (assuming you have confirmed that they're not because of all @) is that lambda function. One way to fix it would be to create a cache of such functions that returns the very same function if file-name is the same. React has something built-in for that. There are ways to do that in Reagent but I don't remember them from the top of my head.

Alex 2020-12-04T05:42:32.338100Z

Oo interesting note about the into and map. I'll take a look at that I ended up just swapping the Reagent ref-setter fn and de-refing the Reagent atom in favor of a re-frame event handler and subscription. And the issues went away

polymeris 2020-12-04T16:14:16.339700Z

Just finished the 7GUIs in reagent, what do you think?

dgb23 2020-12-07T01:44:51.344900Z

Hey I skimmed over it and it looks good! One minor nitpick: on the modal dialog you have:

left: calc(50vw - 100px);
width: 200px;
Now 100px seems to be what we call a “magic number”. what you actually mean is: put the element into the middle and substract half of the width to center it. Now, again for this amount of code this is ofc fine. But it would be cleaner if you used CSS variables for this. So both the width and half of the width is based on the same value. You can also “fix” this by flexing it (justify & align center on the container) or by using transform: translate(-50%, 0) so you don’t need to change multiple values at once. The reason I say this is this: I’ve written a ton of CSS, very much including “badly” structured CSS. Lack of experience, time constraints are my excuse. But I’ve found that structuring it well, if you have the time, is very rewarding in the long run. Especially if you have a back and forth with a designer or client, or when you want to change things.

dgb23 2020-12-07T01:50:12.345100Z

Also I like how the clojure code looks much more structured now

polymeris 2020-12-08T00:26:00.346200Z

I like the transform trick, but I think I'll take the opportunity to learn about css variables.

polymeris 2020-12-08T00:27:29.346400Z

Yeah, I have tried a few times to grok CSS structure, but I guess I really lack the experience to identify what level of abstraction, generalization (or specificity) is appropriate in each case

polymeris 2020-12-08T00:27:32.346600Z

Thanks again!

dgb23 2020-12-04T16:54:58.340Z

I skimmed your CRUD example. My intuition is: I don’t like inline-styles here, or at least not if they could just as much be separated (static) from the components. They add noise to the code so it is a bit harder to read. Also I think your notes/comments are spot on. Refactoring it, so it doesn’t use DOM hacks, and a cleaner data model with ids would be beneficial. Most of your event handlers don’t need to close over the component (panel) but only over your db . So it might be slightly more readable to name (def) them at the top, and just pass in the values you need when registering them in your component.

dgb23 2020-12-04T16:59:35.340200Z

With “most” I mean the ones that do the actual CRUD

polymeris 2020-12-04T17:03:00.340400Z

Thanks! Yeah, I think the CRUD is the one GUI I am least happy with, and that's a good point about the event handlers. Re styles, I'll admit I am not up to date with what's the way to go. Is BEM still considered good practise?

dgb23 2020-12-04T17:08:40.340600Z

BEM is a naming convention and CSS structuring pattern right? I personally advocate for atomic/utility based CSS for most things and then create component based CSS rules with semantic class names if you find repeating patterns and clear “coupling” of CSS properties.

dgb23 2020-12-04T17:09:42.340800Z

But there is no single best way. Just be consistent in your naming and keep it mostly DRY

dgb23 2020-12-04T17:22:26.341Z

I just skimmed over BEM: I really like it and it makes sense. Certainly not a bad way to structure CSS and it is consistent. The only downside would be that you need to “think” a bit more upfront to get the separation right. (Which is why I like to go from atomic and then refactor into components)

polymeris 2020-12-04T17:43:28.341200Z

From what I gather, that means doing something like [:div.flex.flex-column ... instead of [:div {:style {:display :flex, :flex-direction :column}} ...? If I am going to refactor into component-based classes/BEM I think I can skip that step entirely

dgb23 2020-12-04T18:04:16.341400Z

I agree. It’s just a process. But if you see the patterns then you might just as well IMO.

dgb23 2020-12-04T18:04:29.341600Z

esp with such a small scope

polymeris 2020-12-04T19:48:35.341800Z

Gave it a shot: There is probably much more possible DRYing of that CSS, but at least the inline styles don't clutter up the code anymore

polymeris 2020-12-04T19:48:43.342100Z

Thanks for the feedback!