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

Use fake popups instead of system popups #144

Merged
merged 3 commits into from
Dec 7, 2023

Conversation

guyonvarch
Copy link
Member

@guyonvarch guyonvarch commented Dec 7, 2023

Using popups, it was hard to test the kiosk without setting up PlayOS VM.

Aditionally:

  • use only one webview,
  • decorate as dialog when necessary,

There should be no visual changes.

See https://trello.com/c/SJNLN2OJ/1550-playos-dialog-less-settings-and-captive-portal

Tests

  • Using local captive portal on VM, I can click on the message when settings are open.
  • With shed-key, use proxy of external server with basic auth.
networking.firewall.allowedTCPPorts = [ 1234 ];

services.tinyproxy = {
  enable = true;
  settings = {
    Port = 1234;
    Timeout = 600;
    Listen = "0.0.0.0";
    Allow = "12.13.87.my-ip";
    BasicAuth = "user password";
  };
};

Checklist

  • [ ] Changelog updated
  • Code documented
  • [ ] User manual updated

This will allow us to use only one webview, and to decorate it or not
with a dialog, instead of using multiple webviews.
This allows to provide show and hide method, while still preserving the
position of the widget in the tree.

Otherwise, when hiding an element, it’s removed from the tree.
@guyonvarch guyonvarch added the reviewable Ready for initial or iterative review label Dec 7, 2023
Using popups, it was hard to test the kiosk without setting up PlayOS VM.

Aditionally:

- use only one webview,
- decorate as dialog when necessary,
@guyonvarch
Copy link
Member Author

Adapting the last commit message, wrongly saying it was not possible to open the captive portal when settings was open, as this OK on develop.

@guyonvarch
Copy link
Member Author

guyonvarch commented Dec 7, 2023

  • Think about scenario with captive portal + proxy.

@knuton
Copy link
Member

knuton commented Dec 7, 2023

[ ] Think about scenario with captive portal + proxy.

We should expect no difference, as the proxy is set application wide.

Copy link
Member

@knuton knuton left a comment

Choose a reason for hiding this comment

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

Checked

  • proxy on shed stick,
  • captive portal on shed stick,
  • captive portal banner can be interacted with if config dialog is visible,
  • key combos (Ctrl-Shift-F12, Esc) on shed stick work,
  • kiosk running as one window on dev machine.

Nice 👍

@knuton knuton removed the reviewable Ready for initial or iterative review label Dec 7, 2023
@knuton
Copy link
Member

knuton commented Dec 7, 2023

@guyonvarch If I understand the question correctly, there should be no change and therefore no new issue in the interaction between proxy and captive portal checks. Feel free to merge if you agree, or let me know if there is another angle that I may be missing.

@guyonvarch
Copy link
Member Author

guyonvarch commented Dec 7, 2023

We should expect no difference, as the proxy is set application wide.

Exact.

One difference from before is that we take care of authentication for the captive portal as well:

https://github.com/guyonvarch/playos/blob/b59200ef4d608388937b2069e5b7bd20beb9990b/kiosk/kiosk_browser/browser_widget.py#L41-L44

I think it makes more sense now than before, as before we setup the proxy, but it would have failed if it required auth for the captive portal.

@guyonvarch guyonvarch merged commit ce548e9 into dividat:develop Dec 7, 2023
1 check passed
@guyonvarch guyonvarch deleted the dialog-less-webview-2 branch December 7, 2023 15:20
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