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

issue/3281 Added data-order to allow sorting of nav buttons #132

Merged
merged 3 commits into from
Feb 7, 2022

Conversation

oliverfoster
Copy link
Member

part of adaptlearning/adapt_framework#3281

using adaptlearning/adapt-contrib-core#61

Added

  • data-order attribute to navigation bar button to allow sorting

@@ -13,6 +13,12 @@ export default class PageLevelProgressNavigationView extends Backbone.View {
return 'btn-icon nav__btn nav__pagelevelprogress-btn pagelevelprogress__nav-btn';
}

attributes() {
return {
'data-order': (Adapt.course.get('_globals')?._extensions?._pageLevelProgress?._navOrder || 90)
Copy link
Contributor

Choose a reason for hiding this comment

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

Code is defaulting to 90, so should the schemas also use 90 for the default?

Copy link
Member Author

@oliverfoster oliverfoster Feb 3, 2022

Choose a reason for hiding this comment

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

Ach no, I'll set to 0. because float:right inverts the current visual order, reordering these correctly in the dom will currently produce the wrong visual order. These should default to 0 but at least be available to change in a project or when we move to floats in the vanilla theme.

Copy link

@lc-alexanderbenesch lc-alexanderbenesch left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

Choose a reason for hiding this comment

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

👀

@oliverfoster oliverfoster merged commit d75f11f into master Feb 7, 2022
@oliverfoster oliverfoster deleted the issue/3281 branch February 7, 2022 09:53
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.

4 participants