Skip to content
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

fix: only apply bg-brand-primary to SearchHeader if variant=inverse #281

Merged
merged 1 commit into from
Dec 6, 2022

Conversation

adamstankiewicz
Copy link
Member

@adamstankiewicz adamstankiewicz commented Dec 6, 2022

Description

Fixes the issue where there is incorrectly a background color on the SearchHeader component used across 3 MFEs.

Screenshots

Before (Bulk Enrollment)

image

After (Bulk Enrollment)

image

More details

  • Dynamic theming was just recently-ish added to admin portal, so the bg-brand-primary class that was already on the SearchHeader component began to apply a style change once that class name had meaning within the admin portal MFE.
  • It does not affect explore catalog because that MFE does need to support dynamic theming, so the class name there has no styles associated with it.
  • In the learner portal, we use the inverse variant and actually do want the background to be the enterprise's theme color. To keep support for learner portal, but fix for the other MFEs, the bg-brand-primary class will only be applied when variant="inverse", which is only true for the learner portal.

Merge checklist:

  • Evaluate how your changes will impact existing consumers (e.g., frontend-app-learner-portal-enterprise, frontend-app-admin-portal, and frontend-app-enterprise-public-catalog). Will consumers safely be able to upgrade to this change without any breaking changes?
  • Ensure your commit message follows the semantic-release conventional commit message format. If your changes include a breaking change, ensure your commit message is explicitly marked as a BREAKING CHANGE so the NPM package is released as such.
  • Once CI is passing, verify the package versions that Lerna will increment to in the Github Action CI workflow logs.
    • Note: This may be found in the "Preview Updated Versions (dry run)" step in the Github Action CI workflow logs.

Post merge:

  • Verify Lerna created a release commit (e.g., chore(release): publish) that incremented versions in relevant package.json and CHANGELOG files, and created Git tags for those versions.
  • Run the Publish from package.json Github Action workflow to publish these new package versions to NPM.
    • This may be triggered by clicking the "Run workflow" option for the master branch.
  • Verify the new package versions were published to NPM (i.e., npm view <package_name> versions --json).
    • Note: There may be a slight delay between when the workflow finished and when NPM reports the package version as being published. If it doesn't appear right away in the above command, try again in a few minutes.

@codecov
Copy link

codecov bot commented Dec 6, 2022

Codecov Report

Merging #281 (10fa83a) into master (4776949) will increase coverage by 0.03%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #281      +/-   ##
==========================================
+ Coverage   77.46%   77.50%   +0.03%     
==========================================
  Files          34       34              
  Lines         648      649       +1     
  Branches      162      162              
==========================================
+ Hits          502      503       +1     
  Misses        133      133              
  Partials       13       13              
Impacted Files Coverage Δ
packages/catalog-search/src/SearchHeader.jsx 92.30% <100.00%> (+0.64%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4776949...10fa83a. Read the comment docs.

@adamstankiewicz adamstankiewicz merged commit 7f77bfd into master Dec 6, 2022
@adamstankiewicz adamstankiewicz deleted the ags/fix-searchheader-bg-styles branch December 6, 2022 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants