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(ui): Window size setting #1299

Merged
merged 18 commits into from
Oct 17, 2023
Merged

feat(ui): Window size setting #1299

merged 18 commits into from
Oct 17, 2023

Conversation

Flemmli97
Copy link
Collaborator

What this PR does 📖

  • Makes it so window size is remembered and reloaded on app start

Which issue(s) this PR fixes 🔨

@github-actions github-actions bot added Don't merge yet DO NOT MERGE Missing dev review Still needs to be reviewed by a dev labels Oct 9, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Oct 9, 2023

UI Automated Test Results Summary for MacOS/Windows

460 tests   345 ✔️  1h 4m 26s ⏱️
  39 suites  115 💤
    3 files        0

Results for commit 6bb2e28.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 9, 2023

UI Automated Tests execution is complete! You can find the test results report here

@github-actions github-actions bot added the Failed Automated Test This PR is failing Luis's Appium test and needs revised label Oct 9, 2023
@github-actions github-actions bot removed the Failed Automated Test This PR is failing Luis's Appium test and needs revised label Oct 9, 2023
@phillsatellite
Copy link
Contributor

While testing I noticed a bug on MacOS, I was not able to replicate on Windows or Linux.

When you re open the app on MacOS the width will be the same as when you closed app but the height will stretch from the top of the screen to the bottom

Screen.Recording.2023-10-10.at.12.00.22.PM.mov

@phillsatellite phillsatellite added the Changes requested When other dev or QA request a change label Oct 10, 2023
Copy link
Contributor

@sdwoodbury sdwoodbury left a comment

Choose a reason for hiding this comment

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

If I remember correctly, resizing the window emits a large number of events. Saving state every time will hit disk a lot. oh well - don't really have a better way to do it at the moment.

@Flemmli97
Copy link
Collaborator Author

ye unless there is a way to detect when someone stops resizing (which i doubt) i dont think there is a better way.

@Flemmli97 Flemmli97 removed the Changes requested When other dev or QA request a change label Oct 11, 2023
@github-actions github-actions bot added the Failed Automated Test This PR is failing Luis's Appium test and needs revised label Oct 11, 2023
@phillsatellite
Copy link
Contributor

Getting this error when trying to compile on Windows and Mac. Linux was able to compile fine

Screenshot 2023-10-11 at 11 13 43 AM

@phillsatellite phillsatellite added the Changes requested When other dev or QA request a change label Oct 11, 2023
@Flemmli97 Flemmli97 removed the Changes requested When other dev or QA request a change label Oct 11, 2023
@github-actions github-actions bot removed the Failed Automated Test This PR is failing Luis's Appium test and needs revised label Oct 11, 2023
@stavares843
Copy link
Member

missing linter 🔨

@stavares843 stavares843 added the Changes requested When other dev or QA request a change label Oct 11, 2023
@github-actions github-actions bot added the Failed Automated Test This PR is failing Luis's Appium test and needs revised label Oct 12, 2023
@phillsatellite phillsatellite added the Changes requested When other dev or QA request a change label Oct 12, 2023
@github-actions github-actions bot removed the Failed Automated Test This PR is failing Luis's Appium test and needs revised label Oct 12, 2023
@Flemmli97 Flemmli97 removed the Changes requested When other dev or QA request a change label Oct 12, 2023
@Flemmli97
Copy link
Collaborator Author

should we also save the window position? as the position will be default value so depending on app size the window can still go out of the screen

@github-actions github-actions bot added the Failed Automated Test This PR is failing Luis's Appium test and needs revised label Oct 12, 2023
@luisecm
Copy link
Contributor

luisecm commented Oct 12, 2023

Looks like there is any issue when compiling on MacOS and Windows:
image

@github-actions github-actions bot removed the Failed Automated Test This PR is failing Luis's Appium test and needs revised label Oct 13, 2023
@phillsatellite
Copy link
Contributor

should we also save the window position? as the position will be default value so depending on app size the window can still go out of the screen

Would this be hard to do? It might be nice to add but if it might come with a lot of bugs and be a lot of hard work it might be best to hold that off for a later time

@sdwoodbury
Copy link
Contributor

should we also save the window position? as the position will be default value so depending on app size the window can still go out of the screen

Would this be hard to do? It might be nice to add but if it might come with a lot of bugs and be a lot of hard work it might be best to hold that off for a later time

@phillsatellite @Flemmli97
looks like this is easy enough to do once the window position is saved.
image

@Flemmli97
Copy link
Collaborator Author

done. might not work on linux though as the api handling the position does not work right there

@phillsatellite phillsatellite added Waiting for CI Waiting for at least one CI job to complete before merging and removed Missing dev review Still needs to be reviewed by a dev labels Oct 16, 2023
@phillsatellite phillsatellite added QA Tested QA has tested and approved Ready to Merge This PR is ready to merge and removed Don't merge yet DO NOT MERGE Waiting for CI Waiting for at least one CI job to complete before merging labels Oct 16, 2023
@stavares843 stavares843 merged commit 75009aa into dev Oct 17, 2023
@stavares843 stavares843 deleted the window_size_setting branch October 17, 2023 01:15
@github-actions github-actions bot added Failed Automated Test This PR is failing Luis's Appium test and needs revised and removed QA Tested QA has tested and approved Ready to Merge This PR is ready to merge labels Oct 17, 2023
@stavares843 stavares843 removed the Failed Automated Test This PR is failing Luis's Appium test and needs revised label Feb 19, 2024
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.

bug(ui): Remember size of app from when previously logged in
5 participants