Skip to content

Minor cleanups to wasm driver - #5790

Merged
Jacalz merged 10 commits into
fyne-io:developfrom
Jacalz:wasm-driver-fixes
Jul 6, 2025
Merged

Minor cleanups to wasm driver#5790
Jacalz merged 10 commits into
fyne-io:developfrom
Jacalz:wasm-driver-fixes

Conversation

@Jacalz

@Jacalz Jacalz commented Jun 16, 2025

Copy link
Copy Markdown
Member

Description:

This removes some GOOS checks (likely leftovers from desktop driver) which were never going to be true, makes sure we clean up the pending function to run and also includes some minor cleanups.

Checklist:

  • Tests included.
  • Lint and formatter run with no errors.
  • Tests all pass.

@coveralls

coveralls commented Jun 16, 2025

Copy link
Copy Markdown

Coverage Status

coverage: 62.385% (-0.002%) from 62.387%
when pulling c174440 on Jacalz:wasm-driver-fixes
into e99e545 on fyne-io:develop.

Comment thread internal/driver/glfw/window_wasm.go Outdated
@Jacalz
Jacalz requested a review from andydotxyz June 17, 2025 12:46
sdassow
sdassow previously approved these changes Jul 3, 2025

@sdassow sdassow left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Diff looks good to me.

@andydotxyz andydotxyz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice to get tidy-ups, but I think it cuts out code that should be fixed not removed.

modifier := desktopModifier(mods)
var button desktop.MouseButton
rightClick := false
if runtime.GOOS == "darwin" {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not sure about removing this code - although it was not executing it has purpose.
What we really need to do is move it to a runtime check of the client not the system.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ah, sorry. I thought it was just working around inconsistencies in GLFW. Will fix.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I fixed it up and also noticed that support for mouse button key 3 was missing compared to desktop. Added that in as well.


switch btn {
case glfw.MouseButton1:
if rightClick {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As above, ctrl+left on macOS should be right

@andydotxyz andydotxyz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice tidies thanks

@Jacalz
Jacalz merged commit d535ae1 into fyne-io:develop Jul 6, 2025
11 checks passed
@Jacalz
Jacalz deleted the wasm-driver-fixes branch July 6, 2025 11:10
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