-
Notifications
You must be signed in to change notification settings - Fork 81
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
Override constrainToScroll prop default value to false #586
Changes from 1 commit
5d0e433
f1b8ee0
837d6e2
44409f6
ec8cf3b
996d695
b577fa8
f9fcaf0
8ea6972
e358e4b
3d3107a
1b51e44
86a55a3
3f55c9e
98018f4
bcf6fa2
ce80b6a
4b20918
add2f37
5f9fd92
4ad6e9d
4c0850d
e3cfdb3
691b74a
f3690dd
e24ea56
5f25479
b4d4ea2
f8b5ef8
76c72de
8dbb67b
883ab3d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ | |
:z-index="99" | ||
:containFocus="true" | ||
:dropdownPosition="position" | ||
:constrainToScrollParent="constrainToScrollParent" | ||
@close="handleClose" | ||
@open="handleOpen" | ||
> | ||
|
@@ -34,6 +35,11 @@ | |
UiMenu, | ||
}, | ||
props: { | ||
// Override default prop value | ||
constrainToScrollParent: { | ||
type: Boolean, | ||
default: false | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For consistency, I think this needs to default to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are also, 1 or 2 linting issues to take care of. You can fix any linting issues by running There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For changelog build failures, please copy the changelog below(I made a few changes to your changelog from the pr) and add it to the - [#586]
- **Description:** Adds a new prop `constrainToScrollParent ` to `KDropdownMenu` to allow overriding of its popover flipping behavior.
- **Products impact:** Bugfix
- **Addresses:** [#432](https://github.com/learningequality/kolibri-design-system/issues/432)
- **Components:** KDropdownMenu
- **Breaking:** no
- **Impacts a11y:** no
- **Guidance:** Use the `constrainToScrollParent` prop to override the default popover flipping behavior of the `KDropdownMenu` component prop where necessary.
[#586]: https://github.com/learningequality/kolibri-design-system/pull/586 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please be sure to let me know in case you need any support in the process. Thanks again |
||
}, | ||
/** | ||
* An array of options objects, with one object per dropdown item | ||
*/ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we replace this comment with documentation instead
@MisRob, happy to hear your thoughts if this developer friendly language :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That'd be helpful documentation definitely, thank you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the feedback @akolson. I have made the requested changes in the latest commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean I have to submit another PR showcasing the override for the template in Kolibiri rendering the KDropdownMenu component?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think we could start by overriding the value in Kolibri: Facility Settings, where the bug was initially caught. Please see #432 for specifics. You can test your KDS changes locally in kolibri by running
The rest of the overrides will be done as and when its needed.