-
Notifications
You must be signed in to change notification settings - Fork 1
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
Refactor map to sky map #291
Merged
Merged
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
…r flexible icons - Renamed all references of 'Map' to 'Sky Map' to improve clarity and align with feature requirements. - Updated `TopLevelDestination` to support both `ImageVector` and `PainterResource` for icons. - Modified `SKY_MAP` to use a drawable resource `R.drawable.skymap_icon` as its icon. These changes ensure the navigation reflects the updated naming and enable greater flexibility in icon selection for top-level destinations.
- Renamed the navigation route from 'Map' to 'Sky Map' in `NavHost` setup. - Updated the `Screen.SKY_MAP` route to align with the new naming convention. - Refactored the `MapScreen` to be displayed under the `Sky Map` route.
…Map' - Updated the `selectedItem` in `BottomNavigationMenu` to reference `Route.SKY_MAP`. - Ensured consistency with the refactor from "Map" to "Sky Map" across the application. - Adjusted string resources and navigation routes to reflect the new name. This change aligns the navigation destination for the sky map screen with the updated naming convention, improving clarity for users and developers.
- Updated the clickable modifier on the LandingScreen background to navigate to `Screen.SKY_MAP`. - Changed the navigation action from "Map" to "Sky Map" for consistency with updated naming conventions. - Ensured all references to "Map" in the LandingScreen are aligned with the refactored route and screen name.
…hanges - Updated `navigateTo` tests to reflect the renaming of the route from "Map" to "Sky Map". - Adjusted assertions to ensure navigation to `Route.SKY_MAP` instead of the previous "Map". - Verified that the top-level destinations list includes `TopLevelDestinations.SKY_MAP`. - Ensured correct options are set when navigating to the 'Sky Map' route using argument captors.
…d Painter icons - Updated the `BottomNavigationMenu` to dynamically load icons based on their type: - If `iconVector` is provided, display an `Icon` with the specified ImageVector. - If `iconResource` is provided, use a `painterResource` to display a drawable image. - Applied a color tint (white) to ensure visual consistency across all icons. - Adjusted icon size to `34.dp` for better alignment and usability.
- Added a new drawable resource `skymap_icon` to represent the Sky Map feature. - The icon will be used in the bottom navigation menu to visually distinguish the Sky Map destination.
… descriptions - Updated test tags and text descriptions referencing 'map' to 'sky map' to align with the recent renaming.
… descriptions - Updated test tags and text descriptions referencing 'map' to 'sky map' to align with the recent renaming.
Quality Gate failedFailed conditions |
Kenzoud
approved these changes
Dec 18, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job on refactoring the "Map" to "SkyMap", this new title presents the functionality better and the new icon looks good! Approved for merge.
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.
Pull Request Description:
Description:
This PR updates the Bottom Navigation Menu to improve the user experience and visual clarity by introducing a new icon for the Sky Map feature. It also refactors the navigation bar to support both
ImageVector
andPainter
-based icons, ensuring more design flexibility.Changes:
Added Sky Map Icon:
skymap_icon
to theR.drawable
resource folder.Updated Bottom Navigation Menu:
BottomNavigationMenu
to dynamically handle bothImageVector
and drawable resource icons (Painter
).34.dp
) and color tint (white) for consistent appearance.Updated Top-Level Destinations:
TopLevelDestinations
to useiconResource
for the Sky Map tab instead of anImageVector
.Improved Code Flexibility:
This Change:
Issue Reference:
Closes #262