search

w3c / window-management

Window Management API
https://www.w3.org/TR/window-management/
Other
95 stars 25 forks source link

ScreensAdvanced.isExtended mixes system-level concepts with individual screen attributes #52

Closed quisquous closed 2 years ago

quisquous commented 3 years ago

Feedback from @cterefinko.

Screen.isExtended is a system-level concept that says "is the visual workspace extended over multiple screens", but this exists on each individual ScreenAdvanced object as well. Maybe this property could be named something different to clarify this?

quisquous commented 3 years ago

cc @jakearchibald for any thoughts here

quisquous commented 3 years ago

One possible solution here is to split Screen and ScreenAdvanced so that they are separate interfaces rather than to have one derive from the other, and to only have Screen.isExtended. This would also solve the issue of Screen.onchange existing and being inherited by ScreenAdvanced and figuring out whether that fires for changes to ScreenAdvanced when there is a listener on Screen.

michaelwasserman commented 2 years ago

This is reasonable feedback, but I'm unsure if it merits making the ScreenAdvanced interface not inherit from Screen. I think it's feasible to break the inheritance relationship later if needed (taking care when actually removing any attributes exposed), so I don't think this is necessarily something we need to address for the API mvp milestone.

reillyeon commented 2 years ago

I think it's fine for isExtended to be a property on all Screen/ScreenAdvanced instances. The current naming works in our favor here because it can be interpreted as "is the workspace represented by this Screen instance extended by the presence of other screens?" Once you've called getScreens() you probably don't care about that information but it isn't fundamentally wrong to have it on every Screen instance.

I don't think there's any way around the question of firing change events for properties that are added to Screen by the ScreenAdvanced interface. This seems like a fundamental issue with backwards compatibility and the concept of inheritance that we just have to live with. I don't foresee sites actually having an issue with that.

quisquous commented 2 years ago

Thanks for the feedback, Reilly. I think I'll go ahead and close this for now and leave the inheritance in place.