cozy / cozy-client-js

Javascript library to write Cozy applications
https://docs.cozy.io/en/cozy-client-js/README/
MIT License
11 stars 12 forks source link

Intents improvements (onReadyCallback, exposeFrameRemoval) #189

Closed CPatchane closed 7 years ago

CPatchane commented 7 years ago

Two main points:

Edit: Rename exposeRemoving to exposeFrameRemoval and use a data flag instead of a method

y-lohse commented 7 years ago

So API-design wise that's all, the code is good too. I know you plan to update the changelog, but could you please also update the intent documentation?

CPatchane commented 7 years ago

Thanks @y-lohse! Sure, I was hesitating about the exposeRemoving name and your proposals sound better so I will change that πŸ‘ And of course update the docs :)

y-lohse commented 7 years ago

The changes are excellent! However I'm afraid I have a new question (just thought about it, sorry). Right now, the service decides whether or not the iframe gets closed automatically, or if the client has to do it. This means clients must check for a removeIntentFrame property all the time, because they can't know if the service will do it or not. Is this correct? Have you considered doing it the other way around, ie. the client says "Please don't remove the frame when you're done"? Would that make sense?

CPatchane commented 7 years ago

Hum, you're right, I'll think about that to be simpler and less error prone πŸ‘

y-lohse commented 7 years ago

Ok, cool. Sorry again I didn't mention it during the first review, the thought only occured to me later :/

CPatchane commented 7 years ago

@y-lohse That was a very very good point in fact! Better late than never^^ Now it's even better, if the intent find a boolean exposeIntentFrameRemoval in its data provided by the client, the terminate() method will return the removal function with the doc autmatically, no need to use a different method in the service 🎸🎸 See here https://github.com/cozy/cozy-client-js/pull/189/commits/3f56fb88f7c8e02c7fb51e4e8239ad4d405019bc

CPatchane commented 7 years ago

Thanks @y-lohse πŸŽ‰

CPatchane commented 7 years ago

(5) [NUA-3] ETU mes applications n'ont pas besoin de permissions pour afficher Claudy