Skip to content

feat: add set custom primary colors method in initialize flow - #6

Merged
andrey-canon merged 1 commit into
ednx-release/mango.nelpfrom
and/set_custom_colors
Apr 4, 2023
Merged

feat: add set custom primary colors method in initialize flow#6
andrey-canon merged 1 commit into
ednx-release/mango.nelpfrom
and/set_custom_colors

Conversation

@andrey-canon

@andrey-canon andrey-canon commented Feb 2, 2023

Copy link
Copy Markdown

Description:

This allows to set css custom properties, in this case just primary colors, in order to take advantage of this changes eduNEXT/paragon#1

How to test

  1. install this paragon version https://github.com/openedx/paragon/tree/alpha
  2. set mfe api config feat: MFE configuration at runtime openedx/frontend-platform#335
  3. Add to your lms config the following:
    "ENABLE_MFE_CONFIG_API": true,
    "MFE_CONFIG": {
        "CUSTOM_PRIMARY_COLORS": {
            "pgn-color-primary-base": "#AA0000"
        }
    },

Result will be something like this
image

@codecov-commenter

codecov-commenter commented Feb 2, 2023

Copy link
Copy Markdown

Codecov Report

❗ No coverage uploaded for pull request base (ednx-release/mango.nelp@3a4a3fd). Click here to learn what that means.
Patch has no changes to coverable lines.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@                    Coverage Diff                     @@
##             ednx-release/mango.nelp       #6   +/-   ##
==========================================================
  Coverage                           ?   82.13%           
==========================================================
  Files                              ?       38           
  Lines                              ?      957           
  Branches                           ?      175           
==========================================================
  Hits                               ?      786           
  Misses                             ?      159           
  Partials                           ?       12           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@felipemontoya

Copy link
Copy Markdown
Member

As we talked via other channels, I think this is great and it would be even better if this allowed for the optional inclusion of a css variables file.

This way we can continue developing the ways in which to get you the css-variables without the need for a new deployment everytime.

@andrey-canon
andrey-canon force-pushed the ednx-release/mango.nelp branch from 9372490 to 3a4a3fd Compare February 9, 2023 20:22
@andrey-canon
andrey-canon force-pushed the and/set_custom_colors branch 2 times, most recently from 83e21ce to ee0e1aa Compare February 9, 2023 20:27
Comment thread src/config.js Outdated
* @property {string} MFE_CONFIG_API_URL
* @property {string} APP_ID
* @property {string} SUPPORT_URL
* @property {string} CUSTOM_PRIMARY_COLORS

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be an object instead of a string property

Comment thread src/initialize.js Outdated
* priority otherwise the level will be calculated based on primary color by
* using the mix function.
*/
export async function setCustomPrimaryColors() {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function doesn't have any process async then you should remove this world

Comment thread src/initialize.js Outdated
// Configuration
await handlers.config();
await runtimeConfig();
await setCustomPrimaryColors();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Await is not necessary here because the function doesn't return a promise.

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.

5 participants