- Mobile action controls alignment #3925
Conversation
- fix Close/Open app bug
WalkthroughThe changes involve modifications to the layout and properties of UI elements in the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (5)
Ginger/Ginger/Actions/ActionEditPages/ActMobileDeviceEditPage.xaml (4)
13-20: Margin and width adjustments look good.The changes improve the layout consistency. However, consider using a constant or style for the ComboBox width to maintain consistency across the application.
Consider defining a constant or style for the ComboBox width:
<UserControlsLib1:UCComboBox x:Name="xKeyPressComboBox" VerticalAlignment="Top" Width="{StaticResource StandardComboBoxWidth}"/>
37-43: Photo simulation panel improvements look good.The margin adjustment, updated instructions, and addition of the SimulatedPhotoFrame improve the functionality and user guidance of the photo simulation feature.
Consider adding a comment to explain the purpose of the SimulatedPhotoFrame for better code documentation:
<!-- Frame to display the simulated photo --> <Frame x:Name="SimulatedPhotoFrame" Background="White" Margin="-10,10,0,0" Width="670" Height="470"/>
46-56: Authentication simulation panel improvements are beneficial.The margin adjustment, simplified label content, and updated instructions enhance the clarity and usability of the authentication simulation feature. The addition of device-specific behavior notes is helpful for users.
For consistency, consider using the same font size for both instruction labels:
<UserControlsLib1:UcLabel Content="Note: Action will work only with a UFT Lab mobile device and Packed application." FontSize="9"/> <UserControlsLib1:UcLabel Content="Note: Android device will simulate FingerPrint, iOS devices will simulate FaceID" FontSize="9"/>Also, there's a typo in "Android" in the second note. Please correct it.
77-81: Width increase for swipe-related input fields is good, but could be more consistent.The increased width from 100 to 160 for both xSwipeScaleTxtBox and xSwipeDurationTxtBox improves usability by providing more space for input.
For better consistency across the UI, consider using the same width (163) as the press and drag duration fields:
<Activities:UCValueExpression x:Name="xSwipeScaleTxtBox" HorizontalAlignment="Left" VerticalAlignment="Center" Width="163" Margin="57 0 0 0" /> ... <Activities:UCValueExpression x:Name="xSwipeDurationTxtBox" HorizontalAlignment="Left" VerticalAlignment="Center" Width="163"/>Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs (1)
975-975: Consider using null-coalescing operator for concise code.The change looks good as it returns the value of
act.ActionAppPackage.ValueForDriver. However, we can make this more concise by using the null-coalescing operator.Consider refactoring the return statement to use the null-coalescing operator:
- return act.ActionAppPackage.ValueForDriver; + return act.ActionAppPackage.ValueForDriver ?? appPackage;This change would return
act.ActionAppPackage.ValueForDriverif it's not null, otherwise it would return theappPackagevalue. This maintains the current logic while making the code more concise and robust.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
<details>
Files selected for processing (2)
- Ginger/Ginger/Actions/ActionEditPages/ActMobileDeviceEditPage.xaml (1 hunks)
- Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs (1 hunks)
Additional comments not posted (5)
Ginger/Ginger/Actions/ActionEditPages/ActMobileDeviceEditPage.xaml (5)
25-27: Width increase for X and Y input fields is appropriate.The increased width from 100 to 139 for both xX1TxtBox and xY1TxtBox should improve usability by providing more space for input. The consistency between X and Y fields is maintained.
59-61: Application package panel adjustments need verification.The margin adjustment for xAppPnl improves consistency with other elements, which is good.
The width of xAppPackageVE has been significantly reduced from 350 to 205. Please verify that this reduction doesn't negatively impact the usability of the input field. Consider the following:
- Is the new width sufficient for typical application package names?
- Does this change align with any new design guidelines or constraints?
- Is there a specific reason for this reduction that should be documented?
If the width reduction is intentional and doesn't impact usability, please add a comment explaining the reason for this change.
66-71: Width increase for duration input fields is appropriate.The increased width from 100 to 163 for both xPressDurationTxtBox and xDragDurationTxtBox should improve usability by providing more space for input. The consistency between press and drag duration fields is maintained.
Line range hint
1-86: Overall, the UI adjustments improve consistency and usability.The changes in this file primarily involve margin adjustments, width modifications, and content updates that enhance the user interface. Most changes are beneficial, improving the layout consistency and providing clearer instructions for users.
A few minor suggestions have been made for further improvement:
- Consider using constants or styles for common widths to maintain consistency.
- Verify the intentional namespace change from
ActionstoActivities.- Add comments to explain the purpose of new elements like SimulatedPhotoFrame.
- Correct the typo in the Android device note.
- Verify the significant width reduction for the application package input.
- Ensure consistent widths across similar input fields.
These changes align well with the PR objectives of addressing UI-related issues and improving the overall user experience.
32-34: Width increase and namespace change for X2 and Y2 input fields.The width increase to 139 for xX2TxtBox and xY2TxtBox is consistent with the previous X and Y fields, which is good for maintaining visual consistency.
The namespace change from
ActionstoActivitiesfor these controls is noted.Please verify that the namespace change from
ActionstoActivitiesis intentional and consistent with other parts of the codebase. Run the following script to check for any inconsistencies:
Maheshkale447
merged commit 475c424
into
Releases/Official-Release
Thank you for your contribution.
Before submitting this PR, please make sure:
Summary by CodeRabbit
New Features
Bug Fixes
Chores