-
Notifications
You must be signed in to change notification settings - Fork 130
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
IFramePositioning #1302
Comments
I don't see anywhere that we're handling the location info coming from the DA at the moment. It probably needs to be added - although it could go into SessionSotrage instead... Take a look at iFrameChannels (which is the message intended to send the data onto the channelSelector UI along with the current selection and channel metadata) for comparison: FDC3/schemas/api/iFrameChannels.schema.json Lines 35 to 57 in 69a4c08
Note that we're only sending the FDC3/schemas/api/iFrameChannelResize.schema.json Lines 23 to 57 in 69a4c08
But it would seem 'less chatty' to add that info to iFrameHandshake message.. So... I guess the message flow could be something like:
Also @julianna-ciq for feedback as you've implemented this. |
FYI I can make changes today or tomorrow before I disappear on vacation (sans laptop)... Hence, if anyone can have a think on this one and let me know ASAP I can try and get that done ;-) |
I didn't get a chance to bottom this out before your holiday, but here's how I think we should approach this.
... however I think we also need to add
I'm going to have a go at updating my implementation along these lines today |
I'm not convinced that you don't need a drag message. From inside the iframe you don't know your current location, but you do know how it needs to change due to a drag (i.e. the offset to apply). That offset then needs applying to calculate a new position and capping at the edge of the window etc. (drag continues, movement does not). Were you to try and handle it like this you would also need messages from the DA about saved position (3 does not eliminate that) and from the client/app window about its dimensions, which the UI would need to do a load of reasoning about to calculate the styles to set. Thats far from ideal - whereas with the drag approach, you are sending offsets from the dragging while the client code works out the end position and can then ask the DA to persist that info, or it can persist it locally in SessionStorage. In the end you will need a combination of things I think:
|
We might need to come back to it. With a proper proof-of-concept. I think this could eat up several months of development time on its own when you consider different browsers, tabs vs. iframes, different css styling that could be applied on the main document, window zoom, accessibility... I don't think we can just put our finger in the air and come up with the right answer to this, unless it's already been done somewhere. The things from my message above we definitely do need either way. |
The dragging came from a StackOverflow with a nice working example thats easy to test cross-browser and @julianna-ciq had a successful play with an example I believe. The examples are old and simple with no real cross-browser concerns:
Tabs vs. iframes also have no bearing as it's purely a relationship between one DOM (whereever it is) and a single iframe within it. Styling should be heavily namespaced to avoid conflicts. Accessibility is a concern for the individual UI implementation within the window. I think we should try for a workable solution and then refine it. |
Ok I'll put it on the stack. In the meantime, are you good with the above changes? |
Not entirely, as I said I don't think its workable that way round for positioning, but otherwise yes. In particular, I'm fine with the iframe sending an outward message first and to include an initial position and styling to set (although we should probably apply some limits one day in the future), but then drag needs to stay as is based on offsets. The DA should update the styling using the offsets from dragging and a resize message for open and close. |
I think we can defer persisting anything for simplicity. Start by focusing on showing, styling and moving the things! |
@robmoffat @julianna-ciq I've made adjustments to the iframe messages in 6bb5699
|
OK I'll take a look thanks! btw I think I can implement the drag functionality with just my IFrameRestyle message - no need for anything else. I've got a meeting now otherwise I'd have a demo ready for our web meeting. |
I don't think you can or should use restyle to implement dragging - the iframe doesn't know its own coordinates in the window, nor the bounds of the window. It would be better to follow all the other examples of draggable iframes and send offsets to the parent window which can handle the move and keeping the widget within the viewport. |
@robmoffat Adjusted the messages as discussed yesterday in: abcc577 |
Just pulling in your changes - there's still an issue about required properties. #1191 (comment) I think this is trying to be too clever - we should allow people to set the properties they want. for example, if you set right, you might not want to also set left. |
Based on our offline discussion about this @robmoffat @julianna-ciq I still think some CSS properties should be required in the iframeHello message (so that a UI implementation is driving its display size and position). As it stands the Hence, we may be ok here and can close this, or I make some small schema changes to require one of several minimal combos... I remain of the opinion we don't need a let me know your thoughts on closing this out. |
Just my two cents |
@julianna-ciq 100% agree on I think your other suggestions are a good compromise - require at least width/height. @robmoffat you ok with that? |
💯 on the However, I'm already doing this for the intent resolver in the demo:
|
Leaving it without required properties. |
In the
WebConnectionProtocol3HandshakePayload
I can see fields for the intent resolver and channel selector URLs. However, there doesn't appear to be a way for the desktop agent to tell the channel selector what position and size it should occupy on the screen at startup.In the POC I used the following:
The text was updated successfully, but these errors were encountered: