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: Add nav drawer icon override support (fixes #417) #498

Merged
merged 3 commits into from
Mar 4, 2024

Conversation

guywillis
Copy link
Contributor

@guywillis guywillis commented Feb 2, 2024

Fixes: #417

Fix

  • Add support for overriding nav drawer btn icon in config.json
  • Convert nav btn label margin to only apply to labels that are a sibling of an icon
  • Added 4 new sidebar icons

Preview of sidebar icons:
Screenshot 2024-02-07 at 16 52 29

Dependency: adaptlearning/adapt_framework#3513

Copy link
Contributor

Choose a reason for hiding this comment

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

👀

@@ -20,8 +21,12 @@
{{/each}}

<button name="drawer" class="btn-icon nav__btn nav__drawer-btn js-nav-drawer-btn is-position-{{_config._drawer._position}}" data-event="toggleDrawer" aria-expanded="false" aria-label="{{_globals._accessibility._ariaLabels.navigationDrawer}}" data-order="{{_globals._extensions._drawer._navOrder}}" data-tooltip-id="drawer">
Copy link
Member

Choose a reason for hiding this comment

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

Does this not become btn-text if there is no icon?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Technically, it should yes. But I have taken the approach of leaving as is for the time being to reduce potential issues.

If we were to change the button class to btn-text in the navigation and have the theme set up to have a different font for buttons than body, then we could run into an issue whereby the fonts are different between buttons in the same space.

Screenshot 2024-02-19 at 12 29 34

Copy link
Contributor

@joe-allen-89 joe-allen-89 left a comment

Choose a reason for hiding this comment

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

👍 Working nicely

@joe-allen-89 joe-allen-89 merged commit d4a7715 into master Mar 4, 2024
@joe-allen-89 joe-allen-89 deleted the issue/417 branch March 4, 2024 11:44
github-actions bot pushed a commit that referenced this pull request Mar 4, 2024
## [6.45.7](v6.45.6...v6.45.7) (2024-03-04)

### Chore

* Delete .DS_Store (#503) ([f52c863](f52c863)), closes [#503](#503)

### Fix

* Add nav drawer icon override support (fixes #417) (#498) ([d4a7715](d4a7715)), closes [#417](#417) [#498](#498)

### Upgrade

* Bump ip from 1.1.8 to 1.1.9 (#501) ([6dad707](6dad707)), closes [#501](#501)
@oliverfoster
Copy link
Member

🎉 This PR is included in version 6.45.7 🎉

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
Labels
enhancement New feature or request released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

replace hamburger icon in navbar
6 participants