It is up to the rendering plug in to enforce individual input types on read-only. To be honest I'm not using that feature anywhere, so it could still have issues. In terms of the action buttons, each one has an option to be visible or enabled. Are you using all of the standard supplied ui stuff, or do you have customizations?

only standard. I confirmed issue in demo. https://github.com/fulcrologic/fulcro-rad-demo/commit/a11ebf16f9d39ce6c549d56ceaf2d41c5d7a8a8b#diff-977e363c7ab417c367c303770d5e468b1f10b12f63a71038b19a11dece2fe9ee Please check if my other changes are acceptable in the demo: • build in docker and couple of bb tasks to start in seconds. • nrepl server to remotely hack • I plan to add @U0522TWDA troubleshooting plugin too.

Thanks. Probably a regression. I’m pretty sure it worked way back when 😞

@U0CKQ19AQ fo/read-only? true works as expected as function it also works.

fo/read-only? (fn [this]
                (= "view" (-> (comp/props this {})
                              (::uism/asm-id)
                              (get (comp/get-ident this))
                              ::uism/local-storage
                              :options
                              :action)))

(let [{::keys          [read-only-fields]
        read-only-form? ::read-only?} (comp/component-options form-instance)
        master-form       (comp/get-computed form-instance ::master-form)
        master-read-only? (or (some-> master-form (comp/component-options ::read-only?))
                              (= "view" (-> (comp/props form-instance {})
                                            (::uism/asm-id)
                                            (get (comp/get-ident form-instance))
                                            ::uism/local-storage
                                            :options
                                            :action)))]

don’t remember. You can always write a function for it.

I mean, if nothing else you could include the uism local storage in the query-inclusions, so that the table would be in props so you wouldn’t have to do quite as many gynamstics

I would like to change form/read-only? So view! will render read-only form. And this long extraction will be in rad code.

I’m game for patches as long as said changes have cause no breaking changes to existing apps. So yes, if fixing “view” mode is all you want to do, then I’m all for it.

There is a small change in the pull request, that renders form read-only on view! I noticed that form/read-only? is prepared for guardrails >defn. I was wrestling several hours to make action-buttons also configurable with (?! ...), without success, because it would be nice to show only relevant buttons. (form/read-only? form-instance {}) can be used to figure out if form is read-only.

Is there a question in there?

No, just a remark, that you might want to change defn read-only? -> >defn ....

All other changes are required in com.fulcrologic.rad.rendering.semantic-ui.form so changing action-buttons into ?! would require coordinated release of fulco-rad and semantic.

would it have to be coordinated? Wouldn’t the change in sui “just work” either way?

all you’re doing is allowing them to supply more, you’re not requiring it

oh, you saying that you’d want to use the new function in RAD?

I am afraid that it will break controls/standard-control-layout if actions-buttons can also be a function, and it is called from sui. So I thing that in this PR I should make actions-buttons ?! compatible, and that fix sui ...

ah, the action buttons should always be maps. You’d just add a new key to each map called :read-only?, right?

I guess it could be nice to allow action buttons to be a function…but there’s already a way to affect if they are visible and disabled

in fact, if they are buttons, shouldn’t you just use those?

I think we’d be talking about control inputs, not buttons

but those are only on reports, right?

it works now; on view just shows [Done] button fo/action-buttons (fn [form-instance] (if (form/read-only? form-instance {}) [::form/done] (concat [::break ::fix] form/standard-action-buttons)))

but you define those in the controls map, right?

of course

I’m ok with the idea of accepting a function there, but really the design was meant for you to use the visible flag on hat controls map, no?

and those already accept functions…was there a bug/deficiency there?

Here’s a general balance I’m trying to strike flexibility vs. confusion. RAD already encourages you to invent your own options for everything. Everything’s an open map. So, I try my best to limit the options I supply to the minimum required to achieve a goal. In this case the goal is “is this button visible”? By adding this layout tweak of yours, we’re duplicating that functionality. It does add one ability: the ability to reorder the buttons at runtime, but is that ever needed? Does the original visible? flag in the controls map already get you what you need?

Ah, but one thing you are trying to do here which should already be baked in: We should not show the save/reset buttons when the form is read only. But isn’t that a patch to their base control map entries in standard controls?

sorry, I think I’m just now completely understanding what you were complaining about

And now I understand that read-only form should automatically show render without save, undo. Without a function above

😛

yeah, I have the “disabled” setting on those buttons that makes them grey out. So my choice of “show them, but disabled” isn’t matching your desire of “don’t show them”

the docstrings on controls should be better

technically you could :

fo/controls (merge my-controls (enc/map-values (fn [c] (assoc c :visible? f)) form/standard-controls)

but I’d happily take a MR that adds that to the standard controls. Disabled should be for when a read-write form is clean, and visible should be used for read only.

I am going to explore this enc/map-values ... it is new to me.

it’s just a helper in taoensso.encore

there are a lot of useful things in there…I use it all the time.

another one I use a lot is enc/catching , which is a macro that makes a CLJC try/catch without needing the conditional compile stuff on the exception type.

browse through the ns. You’ll find a lot of stupid simple stuff that augments clojure.core nicely

Btw, did you noticed that some-> macro is losing ^js in a few hops and it brakes advanced compiler, because you are using it a lot

oh, no

has that always been the case and it was just discovered, or recent bug?

have you seen me threading js values?

i don't know, probably not. just that there are plenty some-> macros, so I was so confused when my running code crashed.

I mean, no one has reported adv compile issues with Fulcro, and I use it in very large projects and don’t have a problem.

I tend to be very conservative in my use of js code. I tend to make wrappers around calls, partially to just avoid the warning highlights of “unknown member”, so perhaps that is accidentally saving me.

(defn foo [^js obj] (.-foo obj))

and yeah, it actually makes sense, since some-> has to rewrite into a sequence of if with new expressions. It makes sense that the metadata on the sym would not be propagated…it isn’t even a sure-thing to propagate it, so while it is kind of a “gotcha”, I would not necessarily call it a bug, since any macro that rewrites syntax could exhibit a similar problem on interop

So, I guess my habit of writing the wrappers has this additional benefit: any macro that messes with the structure of the code won’t break adv compile in cljs.

intuition :)

well, in my case it was just a desire to avoid all the warnings in the editor about “unknown member” in js objects, so I can’t really claim I was doing anything all that special. I don’t like turning off warnings in the IDE because that then pushes the problem down to other devs. I’d rather move the “false warnings” to a well-known pattern so they are easy to visually understand and not be mixed in with the main logic.

That, and then I can add docstrings to them and not have to keep bouncing to js docs if I get annoyed about that…like I do in the SUI wrappers.

I will have to introduce new public function in forms better name view-action? (defn view? [form-instance] (= view-action (-> (comp/props form-instance {}) (::uism/asm-id) (get (comp/get-ident form-instance)) ::uism/local-storage :options :action))) to write controls like that: ::fix {:type :button :label "Fix" :visible? #(not (form/view? %)) ....

it is also on standard controls

That’s fine. Should this look for the master-form?

I don’t think child forms can independently declare they are read-only. It needs to be the master form, right?

so you might want to use get-computed, and then grab the master form? That way the function will work in child forms properly

did you read this:

(defn read-only?
  "Returns true if the given attribute is meant to show up as read only on the given form instance. Attributes
  configure this by placing a boolean value (or function returning boolean) on the attribute at `::attr/read-only?`.

  The form's options may also include `::form/read-only-fields` as a set (or a function returning a set) of the keys that should
  currently be considered read-only. If it is a function it will only be passed the form instance.

  If the form has a `::form/read-only?` option that is `true` (or a `(fn [form-instance] boolean?)` that returns true) then
  *everything* on the form will be read-only.

  If you use a function for read only detection it will be passed the `form-instance` and the `attribute` being
  checked. You may reach into app state to examine things, but beware that doing so may not dynamically update
  as you'd expect."
  [form-instance {::attr/keys [qualified-key identity? read-only? computed-value] :as attr}]
  [comp/component? ::attr/attribute => boolean?]
  (let [{::keys          [read-only-fields]
         read-only-form? ::read-only?} (comp/component-options form-instance)
        master-form       (comp/get-computed form-instance ::master-form)
        master-read-only? (some-> master-form (comp/component-options ::read-only?))]
    (boolean
      (or
        (?! read-only-form? form-instance)
        (?! master-read-only? master-form)
        identity?
        (?! read-only? form-instance attr)
        computed-value
        (let [read-only-fields (?! read-only-fields form-instance)]
          (and (set? read-only-fields) (contains? read-only-fields qualified-key)))))))

I guess I did intend for it to be possible to declare read only in child, but with master being an override

yes, in my open PR I patched this fn

oh, patched it to look at action?

yes

But we are going to hide buttons only on view!

no, I think the master form itself being read-only makes sense.

of course, subforms should follow

hm. I didn’t remember this being so complicated…I’m reviewing the logic

oh, I see. I had not looked at the PR yet. The only thing you’re patching is that function. I see.

yeah, that should be master-form, not form-instance

I just pushed new ver

yeah, but the action is never on the children

nor is the asm-id

so that function has to go looking for the master form

can you take over and make it bullet proof.

the buttons are only rendered on the master, for that matter

I would have to spin up a form set. I was assuming you were already in a state where you could test things.

it is working in my application

right, all the more reason for you to do the fix I’m suggesting and see that it works.

I think it is:

(defn view-mode? 
  "Returns true if the main form was started in view mode. `this` can be from main form or any subform."
  [this]
  (let [{::keys [master-form]
         :or {master-form this}} (comp/get-computed this)]
    (= view-action (-> (comp/props master-form)
                       (::uism/asm-id)
                       (get (comp/get-ident master-form))
                       ::uism/local-storage
                       :option
                       :action))))

I’d prefer that name, I think, along with the docstring

hm…actually, if you pass the main form to that, it won’t have msater-form in computed…edited

Try that out, and if that’s working, then re-send the PR

thanks

There is a pr, instead of this i used form-instance in forms there are this, this-form and form-instance. Also l did let more readable to me. I can revert to your version.