BSN 4.1 Feedback Required
Closed this issue · 42 comments
I recently updated the BSN code base (click the link for a complete changelog) everybody is invited to test and provide feedback. I'm mostly interested to know any issue with Safari/Chrome on iOS, as I've tested everything on WIndows already.
@fmasa @midzer @cvaize @webstaurantstore @dtognazzini @PaulBGD and anyone else
Thank you and have a good one!
dropdown positioning has improved dramatically! i had many issues with dropdowns in a sidebar, with this new version, it's more consistent to what i get with regular bootstrap 5 and popper.
I upgraded to current master to help testing and I get a javascript error on loading:
Uncaught DOMException: Document.querySelector: '#' is not a valid selector
Error is in the last line of this function:
function querySelector(selector, parent) {
const selectorIsString = typeof selector === 'string';
const lookUp = parent && parentNodes.some((x) => parent instanceof x)
? parent : getDocument();
if (!selectorIsString && elementNodes.some((x) => selector instanceof x)) {
return selector;
}
// @ts-ignore -- `ShadowRoot` is also a node
return selectorIsString ? lookUp.querySelector(selector) : null;
}
Any hints how I can debug this? Site works with BSN 4.0 flawlessly.
Thanks for the feedback. I will test myself, perhaps selectors like #myElement don't work with the querySelector utility. You can post a screenshot of the expanded view of the error in the console.
@jcorporation I've tested ID selectors on my setup and everything is fine, waiting for your confirmation and perhaps some sample code I can test with.
Here is the screnshot:
The file that produces this error: https://github.com/jcorporation/myMPD/blob/bsn41/htdocs/index.html
Found the error:
<a data-phrase="Move an output to this partition" class="dropdown-item featPartitions" data-bs-toggle="modal" href="#" data-bs-target="#modalPartitionOutputs"></a>
Must be:
<a data-phrase="Move an output to this partition" class="dropdown-item featPartitions" data-bs-toggle="modal" href="#modalPartitionOutputs"></a>
Yes, that is a breaking change mentioned with each commit on the v4.1 branch. So we good now?
May I suggest using this markup:
<a data-bs-toggle="modal" href="#modalPartitionOutputs" class="dropdown-item featPartitions" data-phrase="Move an output to this partition"></a>and
<button data-bs-toggle="modal" data-bs-target="#modalPartitionOutputs" class="dropdown-item featPartitions" data-phrase="Move an output to this partition" ></button>Makes a lot more sense to keep consistency with each component markup requirements to use data-bs-target on non-anchor elements, and href for anchor elements only. Correct?
I missed this breaking change. Its now all good for me, many thanks!
Other question: where are the reference to an initialized element are stored? In previous version I can access it via triggering element e.g. el.Popover, el.Modal. In 4.1 I see no such reference anymore.
That is another breaking change, you now have to do BSN.Carousel.getInstance(selector | HTMLElement). Check the updated documentation/demo ;)
Is there a list of breaking changes?
Most important changes are written in the commit linked in the initial v4.1 commit, here is the link. But I will try to find some time to compile a list of most important changes.
Update: after a second thought the introduction of Component.getInstance() method is probably the only real breaking change, all other changes are mostly consistency in terms of TypeScript or parity with the original library, which again, everything is documented with the demo/documentation. Other breaking changes are the utilities from shorter-js but should have little to no effect for any regular BSN developer.
Works all as described!
@jcorporation some questions if you don't mind:
- does your script use dropdown / tooltip / popover? I would like to know if there is any improvement / issue in regards to automatic positioning;
- do you think our BSN master branch is eligible for a stable release? if not, please provide info.
Thanks
Yes, my script uses dropdown and popovers. I have some workarounds in place. I tested now without this workarounds.
There are the same positioning problems as in the old version:
Clipped popover/dropdown (height):
Workaround is to set a overflow:auto and resize the popover-body, same behavior for dropdowns.
Clipped dropdowns (right):
Workaround is to set dynamically the dropdown-menu-end classes.
The new version doesn't deal with dropdown-menu-end class anymore because of RTL support. That is something you need to set based on the context.
This is exactly what my workaround does. I hopped that the improved positioning feature sets the dropdown and popovers position and dimensions too prevent clipping through viewport.
That is true for tooltip / popover, it will try to find the best position so it's always visible within the viewport. However dropdown is much more simple since the .dropdown-menu is contained within the .dropdown element. With other words it might be impossible to code something that is RTL compliant and automatically position the .dropdown-menu with or without the dropdown-menu-end class, believe me I tried, you know what's the reason? One invalidates the other, completely.
The improvements for dropdown are mostly related to RTL as well as more consistent positioning, but again, the dropdown-menu-end class is for you to decide where and when to use.
Why not simply droping in the direction where is the most space? In my first screenshot the popover have more space below but it dropups.
Same with second screenshot, why the dropdown does not align to end? If I do not know the position of the dropdown button at coding time, I must determine the position at runtime and add the class accordingly.
It would be more userfriendly if bsn handle this for me. Or what usecase do I miss?
That's what I have in mind. However, it already does that, exactly showing the dropdown-menu where it has the most space and reposition on window scroll/resize.
If you're using dropleft / dropright and it doesn't have space sideways, it will use either dropdown / dropup position, but it will not get from initial dropdown / dropup to dropleft / dropright as far as I can remember. Maybe here's something we could improve on.
I think you should use dropdown-menu-end class for the dropdown in the second screenshot. It would be consistent with the current scripting and also with the original library and I think shifting to the position of dropdown-menu-end without using the class would invalidate using the class in the first place.
First screenshot: why BSN positions the popover at top and not at bottom. At bottom there are more free space. In the markup there are no direction classes, that forces a dropdup. If the screen is higher it is positioned at the bottom.
Second screenshot: I can not know the position of the dropdown element at coding time, because the button could float to the left side. The position is dependent on the screen size.
Do you have a test site? I wanna have a look.
Not for the popover, but for the dropdown element.
https://spacepirates.jcgames.de/
- click on the search button on the top right, the search dropdown appears
- type "wa" to search
- close the dropdown and reopen it
- if the dropdown does not fit at the bottom it is positioned at top even if at the bottom are more space
That is "4.1.0alpha2", there have been a ton of changes since then.
This is how the latest works, and how it's supposed to work
dropdown-test.mp4
For some reason your test site is using the V4 version of BSN which doesn't include the changes I'm talking about.
Sorry, I updated to the wrong version...
Now it should run the latest code, behavior has not changed. At the bottom there much more space than at the top. Does BSN position on top if on the bottom is not enough space independent which space is bigger?
Your search on the site looks good to me, I've cleared the cache and the dropdown-menu is positioned where I expect it to go, except when you minimize to a mobile width and the dropdown is going into .dropdown position but doesn't adjust position.
That dropdown-menu is pretty large, if you want to use the dropstart, I would suggest giving the dropdown a max-height of about 350px and overflow-y: auto, or simply use .dropdown with the .dropdown-menu-end class.
Initial position is as described. But if the dropdown menu does not fit in the space below it always drops up?
If you want the dropdown-menu to be visible on mobile devices, makes sense to limit it's size somehow, right? There is no code to guess where an element larger than the screen should be displayed into.
Can I force it do always dropdown and not dropup?
Yes, you can disable automatic positioning, check the doc for that. Please clear the browser cache and test again, but I think you need to read careful what I wrote above and try it out. Let me know what you came out with. You might get it to work properly without changing any code, except some CSS.
I tested the different options, but I found no ideal way. What I wanted would be a combined dropdown and dropstart: open the menu at the left side of the button and always dropdown.
But you are right, I can overwrite this positioning with some css code.
If you follow my advice, plus setting your search dropdown-menu min-width: 20rem to @media (min-width: 768) you should have an excellent outcome.
Many thanks for your help. Now I looking for popover positioning.
All other tings I tested: popovers, offcanvas, collapse, modals, carousel works without issues.
Thank you @jcorporation I hope you managed to fix them all. As soon as I'm done with wiki updates, I'm going to push 4.1 to the cloud.
Yes, all works as intended. It was a pleasure for me to test the new version!
Noticed an javascript error. It occurs only in chromium if switched to mobile device view.
It occurs after closing an popover. I do it manually calling the hide() method.
It is reproducible with your demo page.
I can confirm that ba090ff fixes the popover js error.