feat: introduce AlertButtonWithConfig with variant from config#789
Draft
adamstankiewicz wants to merge 1 commit intoopenedx:masterfrom
Draft
feat: introduce AlertButtonWithConfig with variant from config#789adamstankiewicz wants to merge 1 commit intoopenedx:masterfrom
adamstankiewicz wants to merge 1 commit intoopenedx:masterfrom
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #789 +/- ##
==========================================
- Coverage 86.78% 86.04% -0.74%
==========================================
Files 48 49 +1
Lines 1400 1412 +12
Branches 294 299 +5
==========================================
Hits 1215 1215
- Misses 172 184 +12
Partials 13 13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
6 tasks
This file contains hidden or 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
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:
Introduces a new component,
DefaultAlertButton, in thereactmodule. It intends to be used by consuming MFEs in theAlertcomponent'sactionsprop to ensure thatAlert's CTAs can be reasonably configured to rely on a different defaultButtonvariant than is used in the upstream MFE code. For example,@edx/elm-themecalls forHowever, this approach on its own, does not adequately address how the Paragon docs site itself would adhere to a change in the default
Buttonvariant used withAlertwhen previewing@edx/elm-theme.To ensure the overridden alert button variant is applied consistently across all MFEs for a given instance, it may be recommended that operators define a common
env.config(e.g.,env.config.common.js) that is consumed by all MFE-specificenv.configfiles so that MFEs don't need to duplicate the same style overrides in multiple places.The idea here would be that
openedxMFEs that supplyactionsto theAlertcomponent should begin usingDefaultAlertButtonfrom@edx/frontend-platform/reactinstead so the configured button variant override is applied without any additional effort within the MFE. It should be as simple as replacing<Button>with<DefaultAlertButton>in the appropriate places.See more details from the Paragon Working Group meeting on 4/2 here.
Next steps:
openedx.env.configfor such style overrides, considering future use cases beyond the immediateAlert.Merge checklist:
frontend-platform. This can be done by runningnpm startand opening http://localhost:8080.module.config.jsfile infrontend-build.fix,feat) and is appropriate for your code change. Consider whether your code is a breaking change, and modify your commit accordingly.Post merge: