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: Fixing installation issues #10

Merged
merged 1 commit into from
Nov 12, 2020

Conversation

awaisdar001
Copy link
Contributor

@awaisdar001 awaisdar001 commented Nov 11, 2020

I came across issues while integrating brand-edx.org with edx-platfrom.
The first issue was wrong number of arguments for the theme-color function.

Screen Shot 2020-11-11 at 10 01 48 PM

I resolved the issue by passing a single argument to the function call.

Once that issue is resolved the second issue was $form-validation-states is not defined.

$form-validation-states variable is used at https://github.com/edx/brand-edx.org/blob/master/paragon/_overrides.scss#L68 but it was not initialized anywhere in brand-edx.org. I observed that the variable actually exists and was commented-out in _variables.scss See here. https://github.com/edx/brand-edx.org/blob/master/paragon/_variables.scss#L737-L750

This might not be a perfect solution but it worked and I am happy to know if there are other (better) ways to address this.

TNL-7669

@awaisdar001 awaisdar001 requested review from AlasdairSwan, abutterworth and davidjoy and removed request for AlasdairSwan November 11, 2020 17:09
// ),
// $form-validation-states
// );
$form-validation-states: () !default;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

$form-validation-states variable is used at https://github.com/edx/brand-edx.org/blob/master/paragon/_overrides.scss#L68 and it should be defined.

@davidjoy davidjoy changed the title Fixing installation issues fix: Fixing installation issues Nov 12, 2020
Copy link
Contributor

@abutterworth abutterworth left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@awaisdar001 awaisdar001 force-pushed the aj/branding/fix-installation-issues branch from b0d8ce3 to 0705b89 Compare November 12, 2020 14:28
@awaisdar001
Copy link
Contributor Author

Thanks, rebased with fix: ... prefix in the commit message.

@abutterworth abutterworth merged commit 393211c into master Nov 12, 2020
@abutterworth abutterworth deleted the aj/branding/fix-installation-issues branch November 12, 2020 14:38
@edx-semantic-release
Copy link
Collaborator

🎉 This PR is included in version 1.1.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants