Skip to content
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

[OPENMEETINGS-2314] video windows alignment fixes and arrangement modes #95

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

CkNoSFeRaTU
Copy link
Contributor

  • rework video windows arrangements to modes with different presets [OPENMEETINGS-2330]
  • added hotkey for cyclic mode change
  • also realign on video window cleanup and window resize

 - rework video windows arrangements to modes with different presets [OPENMEETINGS-2330]
 - added hotkey for cyclic mode change
 - also realign on video window cleanup and window resize
@CkNoSFeRaTU
Copy link
Contributor Author

Currently in "default" mode user-made resizes are not preserved and will be resized back on next arrange. I should probably need to preserve user resizes in that mode.
Also currently sorting in bottomToTop and topToBottom is different. As it was before. Dunno if we need to make it similar for all modes.

@@ -391,7 +391,7 @@ var Video = (function() {
_resizeDlgArea(hasVideo ? size.width : 120
, hasVideo ? size.height : 90);
if (hasVideo && !isSharing && !isRecording) {
VideoUtil.setPos(v, VideoUtil.getPos(VideoUtil.getRects(VIDWIN_SEL), sd.width, sd.height + 25));
VideoUtil.arrange();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Imagine user moved video windows to preferred position
then someone have restarted his/her video device or exit the room
and all user actions will be reset ....

I believe these 3 calls to arrange shoul be removed

Copy link
Contributor Author

@CkNoSFeRaTU CkNoSFeRaTU May 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if I won't do full rearrangement in "default" mode only? And don't touch old windows in that mode even if they are now out of visible zone after window resize. Will it be acceptable solution? Because I really want to preserve all windows rearrange behaviour in other modes...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this would be OK
IF the only way to switch the mode will be Ctrl+Shift+X i.e. mode will not be switched if windows were re-arranged by hotkey

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. I can rework it so in 0 mode (default) all currently present arrange hotkeys and window arrangements will behave exactly like they are behave now and wouldn't switch to different mode.

About ways to switch mode I'm planning to add settings to room configuration in follow-ups patches. For ability to set different default mode and change resolutions for modes 1 & 2. So they are not hardcoded and can be changed on per room basis. And also add global settings to what set them on automatic room creation.


default:
const v2 = v.find('.video').get(0);
w = v2 && parseInt(v2.style.width, 10);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am I right , 10 cab omitted here?

Copy link
Contributor Author

@CkNoSFeRaTU CkNoSFeRaTU May 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't really understand what you mean by omitted cab... :-( It truncate 'px' to make it an integer...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

full syntax of parseInt is parseInt(str, radix), radix is 10` by default

so parseInt(v2.style.width, 10); === parseInt(v2.style.width);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh... cab = can :-))) No, base is not guaranteed to be 10:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/parseInt

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep :)
I was in one-handed-baby-sitting-mode sorry for typo :(

I believe there are no chances size in px will start with 0 or 0x .... :))

Copy link
Contributor Author

@CkNoSFeRaTU CkNoSFeRaTU May 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will omit it if you so desire. :-)
I just had bad experience with its behaviour in the past, even in non-leading zero cases. Especially on weird platforms like set-top boxes. And always specify it now. Better be safe than sorry. :-)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, let it be :)

}

v.find(VID_DLG)
.dialog('option', 'width', w)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is the method in raw-video doing all this
I guess it can be reused

var w, h, f, r = false;

switch (self.arrangeMode) {
case 1:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer constants with meaningful names here :))

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

Successfully merging this pull request may close these issues.

2 participants