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.

Sign up for GitHub

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
Merged

Fix: Add nav drawer icon override support (fixes #417) #498

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cd203f1
Set up nav drawer btn icon override
guywillis Feb 2, 2024
9a6397e
Removed drawer css icon, converted label margin css
guywillis Feb 2, 2024
1706c34
Added new sidebar icons
guywillis Feb 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion fonts/selection.json
100755 → 100644
View file
Open in desktop

Large diffs are not rendered by default.

Binary file modified fonts/vanilla.woff
100755 → 100644
View file
Open in desktop
Binary file not shown.
Binary file modified fonts/vanilla.woff2
View file
Open in desktop
Binary file not shown.
4 changes: 4 additions & 0 deletions less/_defaults/icons.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,3 +272,7 @@
.icon-youku { &:before { content: "\e9eb"; } }
.icon-youtube-2 { &:before { content: "\e9ec"; } }
.icon-youtube { &:before { content: "\e9ed"; } }
.icon-sidebar-01 { &:before { content: "\e9ee"; } }
.icon-sidebar-02 { &:before { content: "\e9ef"; } }
.icon-sidebar-03 { &:before { content: "\e9f0"; } }
.icon-sidebar-04 { &:before { content: "\e9f1"; } }
8 changes: 2 additions & 6 deletions less/core/nav.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,11 @@ html.is-nav-bottom #wrapper {
order: -1;
}

&.has-label-left &__btn-label {
&.has-label-left .icon ~ .nav__btn-label {
margin-inline-end: @icon-padding / 2;
}

&.has-label-auto .nav__btn-label {
&.has-label-auto .icon ~ .nav__btn-label {
margin-inline-start: @icon-padding / 2;
}

Expand Down Expand Up @@ -111,10 +111,6 @@ html.is-nav-bottom #wrapper {
&__back-btn .icon {
.icon-controls-small-left;
}

&__drawer-btn .icon {
.icon-list;
}
}

// Show nav label at breakpoints
Expand Down
9 changes: 9 additions & 0 deletions schema/config.model.schema
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,15 @@
"right"
]
}
},
"_iconClass": {
"type": "string",
"required": false,
"default": "icon-list",
"title": "Drawer icon class",
"inputType": "Text",
joe-allen-89 marked this conversation as resolved.
Show resolved Hide resolved
"validators": [],
"help": "CSS class name to be applied to the drawer sidebar icon."
}
}
},
Expand Down
6 changes: 6 additions & 0 deletions schema/config.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,12 @@
"left",
"right"
]
},
"_iconClass": {
"type": "string",
"title": "Drawer icon class",
"description": "CSS class name to be applied to the drawer sidebar icon.",
"default": "icon-list"
}
},
"_adapt": {
Expand Down
7 changes: 6 additions & 1 deletion templates/nav.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{{! make the _globals object in course.json available to this template}}
{{import_globals}}
{{import_adapt}}

{{! All links in the nav bar should have an attribute of data-event, navigationView uses this to fire a global event}}
<div class="nav__inner">
Expand All @@ -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

<span class="icon" aria-hidden="true"></span>
{{#if _config._drawer._iconClass}}
<span class="icon {{_config._drawer._iconClass}}" aria-hidden="true"></span>
{{/if}}
{{#if Adapt.course._navigation._showLabel}}
<span class='nav__btn-label' aria-hidden="true">{{_globals._accessibility._ariaLabels.navigationDrawer}}</span>
{{/if}}
</button>

</div>