Presenter console: fix disable button for single slide & some improvements on PC close #10989
Conversation
- there is no motive to disable buttons by default - will adjust button state on need basis Signed-off-by: Darshan-upadhyay1110 <[email protected]> Change-Id: I7d355076969e0049cd9660d94739677073110b08
Darshan-upadhyay1110
requested review from
mcecchetti,
eszkadev and
hcvcastro
| @@ -160,6 +160,11 @@ class PresenterConsole { | |||
| this._visibleSlidesCount = this._presenter.getVisibleSlidesCount(); | |||
| this._previews = new Array(this._getSlidesCount()); | |||
|
|
|||
| if (this._visibleSlidesCount == 1) { | |||
| this._disableButton(this._proxyPresenter.document.querySelector('#prev')); | |||
There was a problem hiding this comment.
just a nitpick: do we need to query for an element each time we want to use it? does it change?
I think you could store reference all the time and just use this.nextButton
Darshan-upadhyay1110
force-pushed
the
private/darshan/fix-PC-for-single-slide
branch
from
32affa5 to
de6e265
Compare
|
'unload' event listener was removed, if user clicks the default 'x', it should not close the presenter console |
|
rewindEffect, dispatchEffect, should return something, please check with Marco to simplifiy a bit =) |
Darshan-upadhyay1110
force-pushed
the
private/darshan/fix-PC-for-single-slide
branch
2 times, most recently
from
c1dfa02 to
549abdb
Compare
I think my last commit got butchered somehow :) i updated that committed again |
|
|
Did you test with doing slideshow again after presenter console was closed? |
Yes i tested that part also verified with Henry regarding that point. I think we are good |
|
So regarding last commit 549abdb i Had a discussion with pedro and here is the point which we agreed on.
Conclusion : Close present in window if we close PC and vice versa I would like to request do not merge this patch yet until i do complete the suggested changes and test the patch with defined test set CC: @eszkadev @hcvcastro |
|
removed approve to not merge by mistake |
- Based on current slide and next slide number we will enable and disable the "Next" & "Prev" action button. Signed-off-by: Darshan-upadhyay1110 <[email protected]> Change-Id: Ibb54e1c86b02fa402374dc231ef28dbd476e9aa6
- In current case User needs to click twice to close the presentation - this patch will help to close the PC and presentation after we do click next on last slide in PC - As a user we should close both PC and main presentation on closing PresenterConsole Signed-off-by: Darshan-upadhyay1110 <[email protected]> Change-Id: Ibe7b11c2bf4438ff7bda46b5a4d520c840561919
Darshan-upadhyay1110
force-pushed
the
private/darshan/fix-PC-for-single-slide
branch
from
549abdb to
54df46e
Compare
|
Tested and ready to review :) |
|
Tested. I works nice |
Darshan-upadhyay1110
deleted the
private/darshan/fix-PC-for-single-slide
branch
Summary
TODO
Checklist
make prettier-writeand formatted the code.make checkmake runand manually verified that everything looks okay