feat: select header and footer #409
Conversation
|
Please let me know of any changes needed, this is my first time committing to an open source project and I am interested in contributing further to this one! |
There was a problem hiding this comment.
The scrolling to bottom does not work smoothly when there is a footer in place.
It is caused by the current implementation of the moveFocusDown(), which seems to use some hardcoded pixel values.
This would also not work if someone uses a different height for the scroll button, so technically this was already a bug.
Some quick tests with the following code seemed to fix this issue.
public moveFocusDown() {
const nativeElement = this.viewport.nativeElement;
nativeElement.scrollBy({ top: 100, behavior: 'smooth' });
const { clientHeight, scrollHeight, scrollTop } = nativeElement;
if (clientHeight + scrollTop >= scrollHeight) {
this.scrollDownBtn.stopEmittingEvents();
}
}
|
One small concern I have is how this will behave from an Accessibility standpoint. Not sure how a screenreader would read content in this areas if it's not technically an option. Also i'm curious if this opens up the component to some kind of bad practice. Might be worth doing a manual spot check on this. Not saying we can't add but we might want to mention in docs that adding anything to complex in these areas might not be accessible or affect accessibility |
good point! |
In regards to opening the component to bad practice, I would argue that we should leave it up to the developer on what content they want to display. A warning around it in the docs would be the best way to go. |
|
I have thought about this implementation further, the issue arises because of the content having the scrolling functionality built into the dropdown and this is more of a bandade than addressing the underlying issue. Would it be better to break out the scrollable area into a separate component that can be used or ignored if a developer wanted to use their own simple scroll with overflow y on the container? It would allow total customization of the layout within the content. I would be interested to hear your opinions on this? Example with custom content; |
CO97
force-pushed
the
feat/select-header-and-footer
branch
from
102581d to
f2f65c1
Compare
PR Checklist
Please check if your PR fulfills the following requirements:
guidelines: https://github.com/goetzrobin/spartan/blob/main/CONTRIBUTING.md#-commit-message-guidelines
PR Type
What kind of change does this PR introduce?
Which package are you modifying?
What is the current behavior?
The current select implementation does not allow for fixed items to be added as a header or footer to the dropdown content.
Closes #400
What is the new behavior?
This change adds flexibility without changing the functionality or structure of the existing implementation by adding two new optional directives for projecting in content to the header/footer of the dropdown.
Does this PR introduce a breaking change?
Other information
A real world example of where this is useful is for a select all or reset buttons to be shown in the dropdown, another example is a count of available items vs selected items.