Opened 8 years ago
Closed 7 months ago
#7548 closed enhancement (maybelater)
bp_button_class needs to know what $container value is to set correct $parent_element
Reported by: | hnla | Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | |
Component: | Core | Keywords: | 2nd-opinion |
Cc: |
Description
This follows on from #7532
With Nouveau able to set args for button elements parent wrappers e.g ul, div etc, nouveau hardcoded the $parent_element for the button or anchor to one of 'li' if $container had a value of 'ul'
The problem arises that third party plugins may filter directly to the button class in BP without any means to know their anchor is rendering in a list construct and thus break that construct with an illegal element in the UL.
Proposal & patch to follow as time allows is to pass into the Nouveau args array that $container value so that the BP class has it available then to run a similar check as in Nouveau to replace the 'div' wrapper with a 'li' one if that $container has a value of 'UL' this way we avoid issues such as the 'User Switcher' plugin has.
Change History (10)
#2
@
8 years ago
- Keywords early added
- Milestone changed from 2.9 to 3.0
It's a core issue in that we re-factored core button class to accept changes to the element but while hardcoding a wrapper element, given Nouveau allows both to be modified we should provide a simple means to allow the button class to check what if any $container value set and switch elements, it seems to me to be something inconsequential for the class to maintain.
However for this to work means I have to re-visit the Nouveau button args to pass $container into the arrays so BP button class has it available. For the moment I'm punting this to 3.0 I'll work up a patch then for it.
Can you create a Nouveau ticket and CC me on it?
Think it was noted there but not sure a ticket on Nouveau makes huge sense, maybe though, if not existing I'll circle back around later.
#3
@
7 years ago
Do we have to do the proposed in this ticket - or can we trust plugin authors to set HTML elements correctly for their extensions?
#6
@
7 years ago
I believe this was the only solution I could arrive at for this potential problem where plugins could assume one thing (div, a) while Nouveau might have changed to a (ul, li) - We can't trust plugin authors as it's unfair for them to have to know, in the example of User Switcher it renders the markup it believes is ok, Nouveau can now upset that assumption. Let me look again at this when I have a moment.
#7
@
7 years ago
Given the amount of open tickets, I am very inclined to not want to spend time fixing this for now until it becomes a problem.
We've got a lot of immediate issues to resolve, rather than worry about potential problems.
I'm not convinced this is a core issue (yet)!
Can you create a Nouveau ticket and CC me on it?