fix: Layouts don't respect orientation on iPad #1225
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's Guide by SourceryThis pull request locks the app orientation to portrait mode on both iOS and macOS. This was achieved by modifying the Info.plist file for iOS, updating macOS project settings, and adjusting shared schemes. Class diagram for main.dartclassDiagram
class MyApp {
-Key? key
+MyApp(Key? key)
+Widget build(BuildContext context)
}
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @samruddhi-Rahegaonkar - I've reviewed your changes - here's some feedback:
Overall Comments:
- It might be worth investigating if there's a way to achieve this in the Flutter code itself, rather than modifying platform-specific files.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
samruddhi-Rahegaonkar
force-pushed
the
fix-ipad-orientation
branch
from
e7775cf to
63ccea7
Compare
Build StatusBuild successful. APKs to test: https://github.com/fossasia/badgemagic-app/actions/runs/13746218460/artifacts/2717500135. Screenshots (Android)
Screenshots (iPhone)
Screenshots (iPad)
|
||||||||||||||||||||||||
-1_home_screen.png?raw=true)
-2_text_badge.png?raw=true){kind=icon}
-3_emoji_badge.png?raw=true){kind=icon}
-4_inverted_emoji_badge.png?raw=true){kind=icon}
-5_saved_badges.png?raw=true)
-6_saved_badges_clicked.png?raw=true)
-7_draw_badge.png?raw=true){kind=icon}
samruddhi-Rahegaonkar
force-pushed
the
fix-ipad-orientation
branch
from
841eda1 to
11320e8
Compare
adityastic
reviewed
Mar 6, 2025
iOS/Runner/Info.plist
Outdated
Comment on lines
64
to
84
| <dict> | ||
| <key>CADisableMinimumFrameDurationOnPhone</key> | ||
| <true/> | ||
| <key>CFBundleDevelopmentRegion</key> | ||
| <string>$(DEVELOPMENT_LANGUAGE)</string> | ||
| <key>CFBundleDisplayName</key> | ||
| <string>Badge Magic</string> | ||
| <key>CFBundleExecutable</key> | ||
| <string>$(EXECUTABLE_NAME)</string> | ||
| <key>CFBundleIdentifier</key> | ||
| <string>$(PRODUCT_BUNDLE_IDENTIFIER)</string> | ||
| <key>CFBundleInfoDictionaryVersion</key> | ||
| <string>6.0</string> | ||
| <key>CFBundleName</key> | ||
| <string>badgemagic</string> | ||
| <key>CFBundlePackageType</key> | ||
| <string>APPL</string> | ||
| <key>CFBundleShortVersionString</key> | ||
| <string>$(FLUTTER_BUILD_NAME)</string> | ||
| <key>CFBundleSignature</key> |
There was a problem hiding this comment.
Wondering why this was changed?
There was a problem hiding this comment.
Thanks, @adityastic . I’ll have a look at this — the change seems unintended.
adityastic
force-pushed
the
fix-ipad-orientation
branch
2 times, most recently
from
0a481f0 to
4c91306
Compare
|
@adityastic Let's rebase this and test :)) |
adityastic
force-pushed
the
fix-ipad-orientation
branch
from
4c91306 to
2c5ece0
Compare
adityastic
force-pushed
the
fix-ipad-orientation
branch
from
2c5ece0 to
18d4b70
Compare
@adityastic There we go ! |
||||||||||||||||||||||||
-1_home_screen.png)
-2_text_badge.png){kind=icon}
-3_emoji_badge.png){kind=icon}
-4_inverted_emoji_badge.png){kind=icon}
-5_saved_badges.png)
-6_saved_badges_clicked.png)
-7_draw_badge.png){kind=icon}
Awesome! Great job! |
||||||||||||||||||||||||
adityastic
force-pushed
the
fix-ipad-orientation
branch
from
18d4b70 to
c395166
Compare
samruddhi-Rahegaonkar
force-pushed
the
fix-ipad-orientation
branch
from
c395166 to
c08ced9
Compare
samruddhi-Rahegaonkar
changed the title
AsCress
reviewed
Mar 9, 2025
| @@ -45,7 +45,6 @@ | |||
| <key>UISupportedInterfaceOrientations~ipad</key> | |||
| <array> | |||
| <string>UIInterfaceOrientationPortrait</string> | |||
| <string>UIInterfaceOrientationPortraitUpsideDown</string> | |||
| @@ -48,7 +56,6 @@ class _DrawBadgeState extends State<DrawBadge> { | |||
|
|
|||
| void _setLandscapeOrientation() { | |||
| SystemChrome.setPreferredOrientations([ | |||
| DeviceOrientation.landscapeLeft, | |||
| import 'package:flutter_screenutil/flutter_screenutil.dart'; | ||
| import 'package:provider/provider.dart'; | ||
| import 'globals/globals.dart' as globals; | ||
|
|
||
| void main() { | ||
| setupLocator(); | ||
| WidgetsFlutterBinding.ensureInitialized(); | ||
| SystemChrome.setPreferredOrientations([ | ||
| DeviceOrientation.portraitUp, |
There was a problem hiding this comment.
Why not both portraitUp and portraitDown, if we are supporting only portrait here ?
| void initState() { | ||
| super.initState(); | ||
| SystemChrome.setPreferredOrientations([ | ||
| DeviceOrientation.landscapeRight, |
There was a problem hiding this comment.
Same for here, why not both landscapeRight and landscapeLeft ?
Also, why do we need this separately in initState(), when we already work with the orientations in didChangeDependencies() ?
Does it make any difference ?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes
-This PR fixes issue #1190 by fixing the required orientation for particular screens on iPad.
Changes
SystemChrome.setPreferredOrientationsfor particular screens.Screenshots / Recordings
Checklist:
constants.dartwithout hard coding any value.Fixes #1190.
Summary by Sourcery