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

feat: create graphical local proxy app #164

Merged
merged 5 commits into from
Jan 17, 2024
Merged

feat: create graphical local proxy app #164

merged 5 commits into from
Jan 17, 2024

Conversation

fortuna
Copy link
Contributor

@fortuna fortuna commented Jan 17, 2024

Create a graphical example local proxy app using Fyne.

You can run it from macOS (needs some libs on Linux) with

go run github.com/Jigsaw-Code/outline-sdk/x/examples/fyne-proxy@latest

Screenshot:
image

/cc @amircybersec

@fortuna fortuna requested a review from jyyi1 January 17, 2024 02:09
Add simple UI

Make it work

Fix

Rename

Improve

Clean up

Clean up

Improve
@daniellacosse
Copy link
Contributor

@fortuna how would you like for me to review this? as production code, or as a prototype?

@fortuna
Copy link
Contributor Author

fortuna commented Jan 17, 2024

@daniellacosse
As a prototype, but I’m curious about what you think would be far from production other than internationalization.

@daniellacosse
Copy link
Contributor

@daniellacosse As a prototype, but I’m curious about what you think would be far from production other than internationalization.

I think my main issue with productionization would be Fyne's lack of screen reader support fyne-io/fyne#1093

This won't be accessible to those with low or no vision - and the internet should be for all!

Copy link
Contributor

@daniellacosse daniellacosse left a comment

Choose a reason for hiding this comment

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

Looks good as a prototype, but feel free to weigh my input as you see fit

func (t *appTheme) Color(name fyne.ThemeColorName, variant fyne.ThemeVariant) color.Color {
switch name {
case theme.ColorNameHeaderBackground:
return color.RGBA{R: 0x00, G: 0x45, B: 0x60, A: 255}
Copy link
Contributor

Choose a reason for hiding this comment

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

Annoying there's no HSL/HSV concept in image/color - i find those much more intuitive to work with


var proxy *runningProxy
startStopButton := widget.NewButton("", func() {})
setProxyUI := func(proxy *runningProxy, err error) {
Copy link
Contributor

@daniellacosse daniellacosse Jan 17, 2024

Choose a reason for hiding this comment

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

generally I like to separate the state logic and the UI render logic (kind of a Model -> View thing), which you've basically done here, but I think my ideal would be something closer to the following... (apologizes if my go code here is incorrect)

type app struct {
  fyne.App
  Events: chan string
  UiState
}

func main() {
  /* <setup code> */
 
  // every time an "event" is pushed to the channel, we handle it and re-render
  // based on the new `app.UiState`
  for event := range app.Events {
    /* <handle event> */

    render(app.UiState);
  }
}

This way we can test both the business logic and the render logic independently, and more easily reuse any UI fragments we come up with (like your makeAppHeader) and reuse the same business logic across different interfaces (CLI/API/UI)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want to explore that too, but it doesn't look like Fyne is set up that way. You may appreciate Gio, which I want to try at some point: https://gioui.org/doc/architecture

Copy link
Contributor

Choose a reason for hiding this comment

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

This reminded me of Redux in front-end frameworks. I wonder if Gio has a better design for separating the UI/business logic.

Copy link
Contributor

Choose a reason for hiding this comment

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

I find redux specifically kinda overcomplicates this concept, but yes it's the same principle

Copy link
Contributor

@jyyi1 jyyi1 left a comment

Choose a reason for hiding this comment

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

It looks great, thanks!

x/examples/fyne-proxy/Icon.png Outdated Show resolved Hide resolved
[Details]
Icon = "Icon.png"
Name = "Local Proxy"
ID = "org.getoutline.sdk.examples.fyne_probe"
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to align the app IDs, for example, in the connectivity app it's org.getoutline.sdk.example.connectivity. The difference is example vs examples. Also, do we really want to put the framework name to the app ID (i.e. fyne_probe vs probe vs probe_app)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we are not publishing these apps, it doesn't matter much.

I'm using fyne_proxy (renamed from probe) to differentiate from the other proxy apps we have, and to draw attention to the fact it's using fyne. I want to write a gio_proxy as well.

As for examples/example, I'm leaning towards examples because it's what we call the folder in the repository.


```sh
go run fyne.io/fyne/v2/cmd/fyne package -os android -appID com.example.myapp
$ANDROID_HOME/platform-tools/adb install ./Local_Proxy_Prototype.apk
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the Local_Proxy_Prototype.apk? Will it be generated by cmd/fyne? Why is it not "fyne-proxy.apk"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It uses the app name. But I removed the Prototype. updated

x/examples/fyne-proxy/main.go Outdated Show resolved Hide resolved
x/examples/fyne-proxy/main.go Outdated Show resolved Hide resolved
Copy link
Contributor Author

@fortuna fortuna left a comment

Choose a reason for hiding this comment

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

I made some tweaks to support dark mode well and emphasize the start button:
image
image

[Details]
Icon = "Icon.png"
Name = "Local Proxy"
ID = "org.getoutline.sdk.examples.fyne_probe"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we are not publishing these apps, it doesn't matter much.

I'm using fyne_proxy (renamed from probe) to differentiate from the other proxy apps we have, and to draw attention to the fact it's using fyne. I want to write a gio_proxy as well.

As for examples/example, I'm leaning towards examples because it's what we call the folder in the repository.

x/examples/fyne-proxy/Icon.png Outdated Show resolved Hide resolved

```sh
go run fyne.io/fyne/v2/cmd/fyne package -os android -appID com.example.myapp
$ANDROID_HOME/platform-tools/adb install ./Local_Proxy_Prototype.apk
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It uses the app name. But I removed the Prototype. updated

x/examples/fyne-proxy/main.go Outdated Show resolved Hide resolved
x/examples/fyne-proxy/main.go Outdated Show resolved Hide resolved
x/examples/fyne-proxy/main.go Outdated Show resolved Hide resolved

var proxy *runningProxy
startStopButton := widget.NewButton("", func() {})
setProxyUI := func(proxy *runningProxy, err error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want to explore that too, but it doesn't look like Fyne is set up that way. You may appreciate Gio, which I want to try at some point: https://gioui.org/doc/architecture

@fortuna fortuna merged commit 51171d8 into main Jan 17, 2024
6 checks passed
amircybersec pushed a commit to amircybersec/outline-sdk that referenced this pull request Jan 22, 2024
@fortuna fortuna deleted the fortuna-fyne branch February 21, 2024 03:58
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.

4 participants