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

Let the implicit sizing handle displaying the button. #1705

Merged
merged 1 commit into from
May 20, 2024

Conversation

bri12415
Copy link
Collaborator

@bri12415 bri12415 commented May 17, 2024

Description

Remove the explicit size params on the copy menu item. Verified on macOS, but am unsure if highlighting code and displaying the copy dialog is supported on mobile?

Type of change

  • Bug fix
  • New sample implementation
  • Sample viewer enhancement
  • Other enhancement

Platforms tested on:

  • Windows
  • Android
  • Linux
  • macOS
  • iOS

Checklist

  • Runs and compiles on all active platforms as a standalone sample
  • Runs and compiles in the sample viewer(s)
  • Branch is up to date with the latest main/v.next
  • All merge conflicts have been resolved
  • Self-review of changes
  • There are no warnings related to changes
  • No unrelated changes have been made to any other code or project files
  • Code is commented with correct formatting (CTRL+i)
  • All variable and method names are camel case
  • There is no leftover commented code
  • Screenshots are correct size and display in description tab (500px by 500px, platform agnostic)
  • If adding a new sample, it is added to the sample viewer
  • Cherry-picked to Main branch (if applicable)

@bri12415
Copy link
Collaborator Author

Screenshot 2024-05-17 at 3 42 32 PM

@GuillaumeBelz
Copy link
Contributor

I guess explicit size was used to avoid issues on mobile devices. Did you try on it?

Copy link
Contributor

@GuillaumeBelz GuillaumeBelz left a comment

Choose a reason for hiding this comment

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

If it's ok on mobile device, can be merged

@bri12415
Copy link
Collaborator Author

bri12415 commented May 18, 2024

I'm not sure how that interaction works on mobile. I can't select the code to copy it, and i'm not sure how I would do a right click to bring up the menu either once the code is selected.

@ldanzinger
Copy link
Contributor

the logic currently is the menu will only display on mouse right click, so probably won't be able to trigger that on mobile

acceptedButtons: Qt.RightButton

@bri12415 bri12415 merged commit 9d0201d into v.next May 20, 2024
@bri12415 bri12415 deleted the bri12415/fixCopyButton branch May 20, 2024 15:51
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.

None yet

3 participants