-
Notifications
You must be signed in to change notification settings - Fork 23
fix: Made the alert-success styling explicit for learning mfe #38
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
Conversation
paragon/_learning.scss
Outdated
| .row.w-100.mx-0.mb-4.border.border-light > .col-12 .alert.alert-success, | ||
| .streak-modal .alert.alert-success { |
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.
That is a bit too specific and can easily break with a small change upstream.
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.
Yes, that's true. I will try to find a way where we can target the specific mfe (learning in this case).
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.
Apparently, there is no way to identify a specific mfe. In my current implementation, I think for streak-modal, there is a very little or no chance that it will be changes in the future. But, for course celebration, I had to do this way as there was no better option I could find. I've also discussed this with @arbirali bhai and he thinks the same.
@DawoudSheraz
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.
@HammadYousaf01 thoughts?
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.
@DawoudSheraz here is the relevant code snippet for Alert component used in CourseCelebration,
https://github.com/openedx/frontend-app-learning/blob/master/src/courseware/course/course-exit/CourseCelebration.jsx#L316
There is no generic parent class I could find that could make it a bit less specific. Your thoughts?
The styling for Alert component with success variant was being picked from _learning.scss file which was disturbing other mfes styling like in authn mfe forgot password page. This PR makes the alert styling explicit to only learning mfe so it won't effect any other mfes as we have separate scss files for each mfe.