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: Inherit _fillNavHeight on content objects, change height calculation (fixes #22) #23

Merged
merged 5 commits into from
Feb 1, 2024

Conversation

swashbuck
Copy link
Contributor

@swashbuck swashbuck commented Jan 31, 2024

Fix #22

Fix

  • Ensures that content objects inherit _fillNavHeight from course.json configuration.

Update

  • Use --adapt-navigation-height for logo height when using fill height
  • Change required FW version to 5.30.3

Testing

See #22

@swashbuck swashbuck added the bug label Jan 31, 2024
@swashbuck swashbuck self-assigned this Jan 31, 2024
@guywillis
Copy link
Contributor

Functionally works as expected.

Perhaps the CSS could be adjusted to use the navigation height css variable

Proposed:

.is-fill &__image {
  height: var(--adapt-navigation-height);
}

Instead of the current:

.is-fill &__image {
  height: @icon-size + (@icon-padding * 2);
}

@swashbuck
Copy link
Contributor Author

Perhaps the CSS could be adjusted to use the navigation height css variable

@guywillis That is an excellent idea. I've made this change.

@swashbuck swashbuck changed the title Fix: Properly inherit _fillNavHeight on content objects (fixes #22) Fix: Properly inherit _fillNavHeight on content objects, change height calculation (fixes #22) Feb 1, 2024
@swashbuck swashbuck changed the title Fix: Properly inherit _fillNavHeight on content objects, change height calculation (fixes #22) Fix: Inherit _fillNavHeight on content objects, change height calculation (fixes #22) Feb 1, 2024
Copy link
Contributor

@guywillis guywillis left a comment

Choose a reason for hiding this comment

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

👍

@swashbuck swashbuck merged commit 101670a into master Feb 1, 2024
@swashbuck swashbuck deleted the issue/22 branch February 1, 2024 16:37
Copy link

github-actions bot commented Feb 1, 2024

🎉 This PR is included in version 3.1.2 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

_fillNavHeight course property is not inherited on content objects
2 participants