Skip to content
This repository has been archived by the owner on May 19, 2020. It is now read-only.

added support for read-only rendering #54

Merged
merged 3 commits into from
Apr 22, 2020
Merged

added support for read-only rendering #54

merged 3 commits into from
Apr 22, 2020

Conversation

kanav-raina
Copy link

Signed-off-by: kanav kanavraina98@gmail.com

Issue #45

Add Support for read only

Changes

Added a read only prop to input fields

Flags

Related Issues

  • Issue #
  • Pull Request #

Signed-off-by: kanav <kanavraina98@gmail.com>
@kanav-raina
Copy link
Author

kindly review @irmerk and @mttrbrts.

@mttrbrts
Copy link
Sponsor Member

Great work @kanav-raina, I've found a few small bugs. Can address these too, please, then I'll be happy to merge?

The following fields are not read-only

  • Drop-downs (e.g. as in the currency code in PaymentObligation)
  • Relationship fields (e.g. as in the Contract Field in PaymentObligation)
  • Primitive fields i.e. fields with type String, Boolean, Double, Array etc. (e.g. accelerometerReadings in the DeliveryUpdate example)

@kanav-raina
Copy link
Author

Thanks @mttrbrts for the feedback. I will try to fix the bugs in Drop-downs, Relationship fields and
Primitive fields

Signed-off-by: kanav <kanavraina98@gmail.com>
@kanav-raina
Copy link
Author

kanav-raina commented Apr 22, 2020

hello @mttrbrts,
I have defined the readOnly props in packages/concerto-ui-react/src/concertoForm.js and I am using disabled={ props.readOnly }.
By default readOnly is not defined for select and button.
We can achieve this with a little css like
select[readonly] {
pointer-events: none;
touch-action: none;
}

But I think defining readOnly in concertoForm and using it as disabled={ props.readOnly } in reactfromvisitor is more easy. If you want the css solution I will be happy to do it
Thanks!

@mttrbrts
Copy link
Sponsor Member

But I think defining readOnly in concertoForm and using it as disabled={ props.readOnly } in reactfromvisitor is more easy. If you want the css solution I will be happy to do it

I agree, however, can we keep the readOnly behaviour on the input fields that support it (such as text input), then use disabled on select and checkBoxes?

Signed-off-by: kanav <kanavraina98@gmail.com>
@kanav-raina
Copy link
Author

kanav-raina commented Apr 22, 2020

Done
kindly review @mttrbrts

@mttrbrts
Copy link
Sponsor Member

Great, LGTM!

I think that there are several areas where we could improve the design (ideas below), but I'm happy to merge this change and open another issue. What do you think?

  • Buttons and Dropdowns could be replaced by a different control when readonly, for example, by a label.
    image
  • Replace all fields by labels when readOnly. Perhaps this is should be a different property altogether?

@jeromesimeon
Copy link
Member

Great, LGTM!

I think that there are several areas where we could improve the design (ideas below), but I'm happy to merge this change and open another issue. What do you think?

  • Buttons and Dropdowns could be replaced by a different control when readonly, for example, by a label.
    image
  • Replace all fields by labels when readOnly. Perhaps this is should be a different property altogether?

Looks nice. Yes for read only, we could probably have something a lot more compact?

@kanav-raina
Copy link
Author

Thanks @mttrbrts and @jeromesimeon for the review.

I think that there are several areas where we could improve the design (ideas below), but I'm happy to merge this change and open another issue. What do you think?

@mttrbrts, Kindly merge this pr and open a new issue and assign that to me. I would be happy to work on it

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

Successfully merging this pull request may close these issues.

None yet

3 participants