Skip to content
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

[staging]remove all the previous select while changing module pos #24260

Merged
merged 3 commits into from Apr 2, 2019

Conversation

bahl24
Copy link
Contributor

@bahl24 bahl24 commented Mar 20, 2019

Pull Request for Issue #24182 .

Summary of Changes

Existing code only removes immediate previous select

Testing Instructions

Clear Cache

Expected result

1 ordering only

Actual result

Ordering unexpectedly adds

@bahl24
Copy link
Contributor Author

bahl24 commented Mar 20, 2019

Not sure if _chzn is particular for my system only or is valid in general.

@bahl24 bahl24 changed the title [staging]remove all the previous select [staging]remove all the previous select while changing module pos Mar 20, 2019
@HLeithner
Copy link
Member

please the last PR making changes to this file https://github.com/joomla/joomla-cms/pull/23052/files

there you see that $el.chosen('destroy') has been used, plz try this.

@bahl24
Copy link
Contributor Author

bahl24 commented Mar 20, 2019

@HLeithner Tried replacing .remove() with .chosen('destroy') but it's not working

@HLeithner
Copy link
Member

I think you have to use:
$("#" + $id).chosen('destroy');

or simply try to restore the old version

var $el = $("#" + $id);
if ($el) {	
  $el.chosen('destroy');
  $el.chosen();
}

@bahl24
Copy link
Contributor Author

bahl24 commented Mar 20, 2019

I think you have to use:
$("#" + $id).chosen('destroy');

I tried with this only. Will try with $el, but reverting it might bring back the issue due to which it was removed.

@bahl24
Copy link
Contributor Author

bahl24 commented Mar 25, 2019

var $el = $("#" + $id);
if ($el) {	
  $el.chosen('destroy');
  $el.chosen();
}

@HLeithner I tried to undo the changes made in https://github.com/joomla/joomla-cms/pull/23052/files, but it doesn't solve the issue and might bring back #22935. Also, is there something wrong in the solution I implemented(.remove())?

@HLeithner
Copy link
Member

if this works maybe its the correct way.

@bahl24
Copy link
Contributor Author

bahl24 commented Mar 31, 2019

@infograf768 @HLeithner Kindly test this

@infograf768
Copy link
Member

@Fedik

@infograf768
Copy link
Member

I have tested this item ✅ successfully on 5c90dda


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

@infograf768 infograf768 added this to the Joomla 3.9.5 milestone Apr 1, 2019
@infograf768
Copy link
Member

@HLeithner
Works fine here whether for site or admin modules.

@alikon
Copy link
Contributor

alikon commented Apr 1, 2019

shouldn't we need the uncompressed/compressed version ?

@HLeithner
Copy link
Member

seams there was never a compressed version...

@alikon
Copy link
Contributor

alikon commented Apr 1, 2019

maybe it's time to do it 🙂 or matter for another pr

@HLeithner
Copy link
Member

another Pr would be better, I would like to merge this tomorrow if we get a second test.

@alikon
Copy link
Contributor

alikon commented Apr 1, 2019

I have tested this item ✅ successfully on 4667c8c


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

@joomla-cms-bot joomla-cms-bot removed this from the Joomla 3.9.5 milestone Apr 1, 2019
@ghost
Copy link

ghost commented Apr 1, 2019

Status "Ready To Commit".

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Apr 1, 2019
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Apr 1, 2019
@bahl24
Copy link
Contributor Author

bahl24 commented Apr 1, 2019 via email

@bahl24
Copy link
Contributor Author

bahl24 commented Apr 2, 2019

@HLeithner @Fedik Updated, kindly test

@HLeithner
Copy link
Member

@alikon and @infograf768 can I get new test from you? I would like to merge this for todays RC

@HLeithner HLeithner added this to the Joomla 3.9.5 milestone Apr 2, 2019
@HLeithner HLeithner merged commit 5d707da into joomla:staging Apr 2, 2019
@HLeithner
Copy link
Member

thx

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.

None yet

6 participants