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

Adds support for query parameter "hideMenu" (attempt #2) #171

Merged
merged 1 commit into from
Feb 19, 2021

Conversation

RussTedrake
Copy link
Contributor

@RussTedrake RussTedrake commented Feb 17, 2021

This adds the ability to load a board with the menu hidden; it provides a form of "read-only" mode, but the key bindings are still active.
Use with, e.g.:
http://localhost:5001/boards/qYAZyGdym6HqjnfaGI4yNtpdx0IMHd9qp6kaJPwbJqE-?hideMenu=true

The alternative, proposed in #116, was to remove the menu completely, but this would require more substantial changes to board.js (which hangs on "Loading..." if the menu is removed).
Related to #150
Related to #116

By opening a pull request, I certify that I hold the intellectual property of the code I am submitting, and I am granting the initial authors of WBO a perpetual, worldwide, non-exclusive, royalty-free, and irrevocable license to this code.

@lovasoa
Copy link
Owner

lovasoa commented Feb 17, 2021

Could you add tests for that code?

@RussTedrake
Copy link
Contributor Author

RussTedrake commented Feb 17, 2021

Adding coverage to your existing test that confirms it doesn't break was straightforward.

I tried adding additional tests to send the browser to variants of the url. But I wasn't successful. You only have the one test right now, and something about the browser setup/teardown was, I think, limiting me. (But I haven't used nightwatch before, so it could be my error).

Even copying the contents of testBoard again

function testBoard(browser) {
    var page = browser.url(SERVER + '/boards/anonymous?lang=fr&hideMenu=false')
        .waitForElementVisible('.tool[title ~= Crayon]') // pencil
    page = testPencil(page);
    page = testCircle(page);
    page = testCursor(page);
    page.end();

    var page = browser.url(SERVER + '/boards/anonymous?lang=fr&hideMenu=false')
        .waitForElementVisible('.tool[title ~= Crayon]') // pencil
    page = testPencil(page);
    page = testCircle(page);
    page = testCursor(page);
    page.end();
}

fails with

2021-02-17T13:23:23.452Z	saved board	{"name":"anonymous","size":747,"delay_ms":1}
 Error while running .navigateTo() protocol action: An unknown error has occurred.

2021-02-17T13:23:24.122Z	saved board	{"name":"anonymous","size":747,"delay_ms":1}
✖ Timed out while waiting for element <.tool[title ~= Crayon]> to be present for 5000 milliseconds. - expected "visible" but got: "not found" (5010ms)
    at Object.testBoard (/home/russt/tmp/whitebophir/tests/integration.js:103:10) 


FAILED: 1 assertions failed and  14 passed (9.454s)
_________________________________________________

TEST FAILURE:  1 assertions failed, 14 passed (12.037s)

 ✖ integration
 – testBoard (9.454s)
   Timed out while waiting for element <.tool[title ~= Crayon]> to be present for 5000 milliseconds. - expected "visible" but got: "not found" (5010ms)
       at Object.testBoard (/home/russt/tmp/whitebophir/tests/integration.js:103:10)

I really wanted to do something like this:

function testHideMenu(browser) {
    browser.url(SERVER + '/boards/anonymous?lang=fr&hideMenu=true').waitForElementNotVisible('#menu');
    browser.url(SERVER + '/boards/anonymous?lang=fr&hideMenu=false').waitForElementVisible('#menu');
}

Those tests pass if I replace your current test, but not in addition to the current test.

@RussTedrake
Copy link
Contributor Author

Ah. it was the page.end() that was tearing things down. I've pushed my best version.

This adds the ability to load a board with the menu hidden; it provides a form of "read-only" mode, but the key bindings are still active.
Use with, e.g.:
http://localhost:5001/boards/qYAZyGdym6HqjnfaGI4yNtpdx0IMHd9qp6kaJPwbJqE-?hideMenu=true

The alternative, proposed in lovasoa#116, was to remove the menu completely, but this would require more substantial changes to board.js (which hangs on "Loading..." if the menu is removed).
Related to lovasoa#150
Related to lovasoa#116

Adds support for query parameter "hideMenu"

This adds the ability to load a board with the menu hidden; it provides a form of "read-only" mode, but the key bindings are still active.
Use with, e.g.:
http://localhost:5001/boards/qYAZyGdym6HqjnfaGI4yNtpdx0IMHd9qp6kaJPwbJqE-?hideMenu=true

The alternative, proposed in lovasoa#116, was to remove the menu completely, but this would require more substantial changes to board.js (which hangs on "Loading..." if the menu is removed).
Related to lovasoa#150
Related to lovasoa#116
@lovasoa
Copy link
Owner

lovasoa commented Feb 17, 2021

Ok, I'll try to test that more carefully than last time, and I'll merge if everything is ok.

@RussTedrake
Copy link
Contributor Author

Thanks.

@lovasoa lovasoa merged commit b5d081f into lovasoa:master Feb 19, 2021
@lovasoa
Copy link
Owner

lovasoa commented Feb 19, 2021

Thanks to you for contributing :)

@RussTedrake RussTedrake deleted the hideMenu branch February 20, 2021 01:25
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