Project: Web Components for FHIR Resources

toolkit
gsoc2018-project
gsoc2018

(Subhashinie) #84

:open_mouth: Totally missed it. I’ll correct my components and improve the unit tests as well. Thanks :slight_smile:


(Subhashinie) #86

Regarding the
on-input="${e => this.value[index].district = e.target._input.value}"
line, how about NOT having it in the sub components of a resource, but collecting all the inputs in the main resource component? Don’t you think that it can reduce the code even more, makes things efficient and stops rendering every time the value is updated?


(Saptarshi Purkayastha) #87

I don’t think it is a good idea because that will mean that the internal elements’ value property will not be synced with the input value. Thus, I think the smallest unit element should have the on-input listener function that updates the element’s value property.


(Saptarshi Purkayastha) #88

@lehone @tenas97 @ivange94 @judywawira @namratanehete - can you guys please also review this, since your projects could also reuse this pattern, if you are using polymer. Please provide any comments on the MR discussion before I will merge this in the next couple of days.


(Parvati Ravindranathan Naliyatthaliyazchayil) #90

@SuKSW The component pattern that I had in MR follows pattern like: each of smaller text-fields has their own input value which goes up into the main “value” for that component which is this.value. This gets updated each time if you are reusing this component in other bigger component. The other bigger component again takes values from each of its fields and takes it to ITS this.value. That is how the value is propagated to higher resource. Smallest change will also be propagated to the bigger component. One good example can be found in fhir-human-relation component where other components like name and address were reused and whole fhir-human-relation component will be part of main resource Patient. I am not sure the term “dependency” was used referring to re-usability of one component in other. This may not answer your question but would at-least help you understand how the value is propagated from smallest level to whole resource in the MR pattern. Pls correct me if I am wrong @sunbiz


(Subhashinie) #92

@parumenon1 two way binding is not supported in lit elements. I also had a similar plan like yours. Then it changed. Remember this?

@parumenon1 even if you explicitly notify the top most element using some method, calling “on-input” on every text field will make the top most element re-render every time the user types in each textfiled. (according to the Patient specification the component can have more than 30 textfields -> therefore 30 odd on-inputs -> will re-render more than 30 times <- because the elements are re-rendered every time its properties change)

Going through the code and completely understanding the functionality is an extremely tough job. Even I (who was building very similar components with you) had to go through the code several times to understand what you were trying to do and notice what I mentioned above.

@sunbiz shall we build the components without the
on-input="${e => this.value[index].district = e.target._input.value}" line? and collect user inputs at the resource level?


(Saptarshi Purkayastha) #93

@SuKSW I tried to see if the _render or _didRender was called on each on-input event, but it wasn’t. So I don’t think it actually gets called like you were worrying. I don’t know why, but in _didRender when the value is set, the render does get called. So, I think it’s something they did in lit-element because if they start re-rendering from changes in _render it will become a never-ending cycle.

The main benefits of doing the way @parumenon1 did it (or what you initially showed us), is that the value property is in sync and any developer who uses the element/component only has to think about reading the value property of the main element, and not go to the _input or whatever inner element.


(Subhashinie) #94

Seems like it does the same in _render as well. When ever a property is updated _render also seems to get called. I tested using the following code :


import {LitElement, html} from '@polymer/lit-element';  
import {Textfield} from "@material/mwc-textfield";

class ReRendering extends LitElement { 
    constructor() {
        super();    
        this.value = "";  
    }

	static get properties() {
		return {
		    value: String
		}
	};

	_render({value}) {
        return html`
            ${console.log(this.value)}
            <`mwc-textfield on-input=${e => this.value = e.target._input.value}><`/mwc-textfield>
        `;
	}
}

window.customElements.define('re-rendering', ReRendering);

Following is a screenshot:

The element gets rerendered for every letter that is entered into the textfield. I think “on-input” is meant for place like filtering, so that the user will know whether the query entered is going to give what is needed.


(Parvati Ravindranathan Naliyatthaliyazchayil) #95

@sunbiz there were four example templates given for PWA i.e., Books, Flashcards, Shop and Hacker news(https://polymer.github.io/pwa-starter-kit/setup/). Books and Flashcards had few features that we needed. Flashcards have:

  1. pattern for storing state in local Storage
  2. pattern for loading local data json files.

Books have:

  1. pattern for re-fetching data when the network state changes
  2. pattern for integrating Google Sign-In

I am still not sure if I have to use flashcards or books to at least start off. Due to pattern of state storage in local storage i think i should start with flashcard. I would use features from other templates as per need. But to just start, what do you suggest me to start with-flashcard or books?


(Saptarshi Purkayastha) #96

This is only for this elements render. I thought we were discussing render for the inner elements, no? That’s not called.

Can we use lost focus or something other then? I still think we should update the value in each component.

Ok, I suggest to use books then. I think Redux and network state change is useful for our EHR use-cases.


(Parvati Ravindranathan Naliyatthaliyazchayil) #97

@sunbiz Sure. I will start off using books template Thank you!


(Parvati Ravindranathan Naliyatthaliyazchayil) #98

@sunbiz, @namratanehete, @judywawira Will it be fine if I start now with polymer starter kit and make a end to end work flow for one resource and then move to progressive web app after first evaluation ?


(Subhashinie) #99

I was thinking about the parent element being re-rendered number of times, because its value gets updated every time a child or a grandchild is updated, due to propagation.

Considering a lost locus might solve the problem :smiley: While syncing the property -> value. How about rather than calling onblur on the child element, having a onblur handling method for the element itself? (can be done setting onblur on the div that is returned)

Then, as the user finishes changes within an element and moves on to another, the previous element will update its “value”. This would only update an element’s “value” only once - when it loses focus. As for the top most resource element - the “value” would be updated when the user clicks submit, and set that “value” as POST data.

@sunbiz @parumenon1 @namratanehete @judywawira Do you think this would raise any issues?


(Saptarshi Purkayastha) #100

@SuKSW The whole idea of building components/custom elements is reusable. If the smallest unit element’s value property is not in sync with the value (in FHIR’s case the JSON) of the element that encompasses it, then reusable components are not meeting their purpose.

@SuKSW, can you please finish all your elements and send MRs. You were going to rework and send them separately before the phase 1 evaluation, and now we are at phase 2 evaluation.


(Saptarshi Purkayastha) #101

Fine with me… infact, I suggest that you use plain HTML pages that assemble your elements. You can then work on creating a pwa app, like the books app that they provide as a pattern.


(Subhashinie) #102

@sunbiz @parumenon1 taking back what I told here, calling onblur just on the child elements seems to be enough. The rerendering will not occur as long as the “value” is an array or and object (since we are only editing it and not reassigning). I’ll send the MRs soon as possible.


(Namratanehete) #103

@parumenon1 Even I am fine with it and agree with @sunbiz


(Subhashinie) #104

@sunbiz I created the MR including on-blur and a few other changes. Will be giving a detailed explanation on every line of the code, as a blog post. Please give me feedback on the MR: https://gitlab.com/librehealth/lh-toolkit-webcomponents/merge_requests/40


(Subhashinie) #105

This post explains the reasons for the changes.

This (https://gist.github.com/SuKSW/b7ae5517bc39f30425f9dd99757d31aa) is a commented version of the code.

Extremely sorry for the delay.


(Parvati Ravindranathan Naliyatthaliyazchayil) #106

My apologizes for posting this post earlier on the AMIA proposal group and not here. @sunbiz, @namratanehete, @judywawira, Here is the link to WIP progressive-web-app repo at gitlab : https://gitlab.com/parumenon.pm/lh-toolkit-app.git. The demo will be ready soon.