-
Notifications
You must be signed in to change notification settings - Fork 47
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
Conversation
Add simple UI Make it work Fix Rename Improve Clean up Clean up Improve
@fortuna how would you like for me to review this? as production code, or as a prototype? |
@daniellacosse |
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! |
There was a problem hiding this 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
x/examples/fyne-proxy/main.go
Outdated
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} |
There was a problem hiding this comment.
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) { |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this 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/FyneApp.toml
Outdated
[Details] | ||
Icon = "Icon.png" | ||
Name = "Local Proxy" | ||
ID = "org.getoutline.sdk.examples.fyne_probe" |
There was a problem hiding this comment.
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
)?
There was a problem hiding this comment.
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/README.md
Outdated
|
||
```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 |
There was a problem hiding this comment.
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"?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
x/examples/fyne-proxy/FyneApp.toml
Outdated
[Details] | ||
Icon = "Icon.png" | ||
Name = "Local Proxy" | ||
ID = "org.getoutline.sdk.examples.fyne_probe" |
There was a problem hiding this comment.
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/README.md
Outdated
|
||
```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 |
There was a problem hiding this comment.
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
|
||
var proxy *runningProxy | ||
startStopButton := widget.NewButton("", func() {}) | ||
setProxyUI := func(proxy *runningProxy, err error) { |
There was a problem hiding this comment.
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
Create a graphical example local proxy app using Fyne.
You can run it from macOS (needs some libs on Linux) with
Screenshot:
/cc @amircybersec