Skip to content

Conversation

@brianteeman
Copy link
Contributor

Chosen is not accessible so we shouldnt use it unless it offers something useful.

In the search component in the front end there are potentially two lists Ordering and display #

Neither of these require or benefit from chosen so this simple PR returns them to regular and accessible lists

Pull Request for Issue #24469

Chosen is not accessible so we shouldnt use it unless it offers something useful.

In the search component in the front end there are potentially two lists Ordering and display #

Neither of these require or benefit from chosen so this simple PR returns them to regular and accessible lists
@zwiastunsw
Copy link
Contributor

I have tested this item ✅ successfully on aa618b8


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/24623.

@Quy
Copy link
Contributor

Quy commented Apr 17, 2019

I have tested this item ✅ successfully on aa618b8


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/24623.

@Quy
Copy link
Contributor

Quy commented Apr 17, 2019

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/24623.

@joomla-cms-bot joomla-cms-bot added RTC This Pull Request is Ready To Commit and removed PR-staging labels Apr 17, 2019
@HLeithner
Copy link
Member

Whats the reason to remove this without needs? The only thing what could happen is to break sites that expect chosen.

@brianteeman
Copy link
Contributor Author

It is removed from this page because chosen breaks accessibility and we dont need any of the added features of chosen for the select lists here.

It cannot break an existing site

@ghost ghost added the PR-staging label Apr 18, 2019
@ghost ghost changed the title [staging] Remove chosen from com_search Remove chosen from com_search Apr 18, 2019
@wilsonge
Copy link
Contributor

We don't in this view but the long standing opinion is removing this can affect ssites that were using it on other pages without the include. This was how we ended up basically rolling out chosen into every view in the backend (as well as the consistency argument). I agree with @HLeithner that this is something not to be fixed in 3.x sadly

@brianteeman
Copy link
Contributor Author

We don't in this view but the long standing opinion is removing this can affect sites that were using it on other pages without the include.

I don't follow this logic. We are talking about a component so how it can affect other pages that are not com_search

@wilsonge
Copy link
Contributor

wilsonge commented Apr 23, 2019

No it doesn’t affect other pages but it does affect modules and plugins on that page

For the record I disagreed with this logic at the time. But it’s long been made standard. And especially given we should be trying to 3.x towards more support only mode in anticipation of 4.0 it doesn’t make sense to make changes like this

@brianteeman
Copy link
Contributor Author

in which case the user can create a template override

@zwiastunsw
Copy link
Contributor

@wilsonge : What kind of plugin? For which modules? Some example? Specifically? It was a very bad code!

@Fedik
Copy link
Member

Fedik commented Apr 25, 2019

it was introduced as a fix #1653 for some issue, which no one know anymore
it seems was just a "styling"

@HLeithner
Copy link
Member

I'm closing this because of the statements above not breaking user templates.

@HLeithner HLeithner closed this Jun 5, 2019
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jun 5, 2019
@brianteeman brianteeman deleted the com_search_chosen branch June 5, 2019 15:56
@brianteeman
Copy link
Contributor Author

Shame that accessibility doesnt matter

@brianteeman brianteeman restored the com_search_chosen branch June 7, 2019 06:30
@HLeithner HLeithner reopened this Jun 12, 2019
@Quy
Copy link
Contributor

Quy commented Jun 17, 2019

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/24623.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jun 17, 2019
@HLeithner HLeithner merged commit bc3cf75 into joomla:staging Jun 17, 2019
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jun 17, 2019
@HLeithner HLeithner added this to the Joomla 3.9.9 milestone Jun 17, 2019
@HLeithner
Copy link
Member

Thx

@brianteeman
Copy link
Contributor Author

thanks

@brianteeman brianteeman deleted the com_search_chosen branch June 17, 2019 06:02
wilsonge added a commit to joomla-extensions/search that referenced this pull request Aug 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants