feat: Update button style to new Elm Theme #32
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
omerhabib26
requested review from
farhan-arshad-dev,
HamzaIsrar12,
k1rill and
dixidroid
- Define generic button style for the whole app - Remove the custom/random button initialization - Make dialog's buttons appearance close to native fix: LEARNER-10084
- Apply elm theme on Modals and Dialogs Alerts - Optimise Code fix: LEARNER-10083
omerhabib26
force-pushed
the
2U/omer/LEARNER-10084
branch
from
3dcdab4 to
d9a9235
Compare
dixidroid
requested changes
Sep 30, 2024
| @@ -204,7 +204,7 @@ private fun CourseVideosUI( | |||
| } | |||
|
|
|||
| var isDownloadConfirmationShowed by rememberSaveable { | |||
| mutableStateOf(false) | |||
| mutableStateOf(true) | |||
There was a problem hiding this comment.
What is the point of this change? Right now the download confirmation dialog now appears every time the user opens a course. Should be false by default
There was a problem hiding this comment.
updated by mistake, reverted.
| modifier: Modifier = Modifier.fillMaxWidth(), | ||
| text: String = "", | ||
| onClick: () -> Unit, | ||
| enabled: Boolean = true, | ||
| textColor: Color = MaterialTheme.appColors.primaryButtonText, | ||
| backgroundColor: Color = MaterialTheme.appColors.primaryButtonBackground, | ||
| backgroundColor: Color = Color(0xFFED5C13),//MaterialTheme.appColors.primaryButtonBackground, |
There was a problem hiding this comment.
maybe it would be better to move the color to resources?
farhan-arshad-dev
requested changes
Oct 3, 2024
There was a problem hiding this comment.
Need product approval for the design changes on the following files:
- Fullscreen(error message).
- ChapterEndDialogScreen
- SetupCourseAccessErrorButtons
- CourseOutlineScreen
- NotSupportedUnitFragment
- DashboardGalleryView(Find new course button)
- DiscussionThreadsFragment
- CalendarAccessDialogFragment
profile/src/main/java/org/openedx/profile/presentation/settings/SettingsScreenUI.kt
Outdated
Show resolved
Hide resolved
profile/src/main/java/org/openedx/profile/presentation/settings/SettingsScreenUI.kt
Outdated
Show resolved
Hide resolved
core/src/main/java/org/openedx/core/presentation/dialog/alert/ActionDialogFragment.kt
Outdated
Show resolved
Hide resolved
core/src/main/java/org/openedx/core/presentation/dialog/alert/InfoDialogFragment.kt
Outdated
Show resolved
Hide resolved
core/src/main/java/org/openedx/core/presentation/dialog/appreview/AppReviewUI.kt
Outdated
Show resolved
Hide resolved
dashboard/src/main/java/org/openedx/courses/presentation/DashboardGalleryView.kt
Show resolved
Hide resolved
- Update Button's prefixe
Have we received product approval for these changes? Also need to address 2 comments, other than that ready to be approved. |
seem fine to me |
farhan-arshad-dev
approved these changes
Oct 9, 2024
farhan-arshad-dev
approved these changes
Oct 10, 2024
dixidroid
approved these changes
Oct 10, 2024
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.
Description:
fix: LEARNER-10083 + LEARNER-10084
Ref: https://www.figma.com/design/q6z6cpJsbBBcnPZLPS7KEu/Marketplace-Visual-Design?node-id=3932-2331&t=C9RFjYFEPRxwC2Hc-0