Fix https://github.com/joomla/joomla-cms/issues/15569 #15573

Merged
merged 1 commit into from Apr 26, 2017

Conversation

Projects
None yet
7 participants
@fevangelou
Contributor

fevangelou commented Apr 26, 2017

Pull Request for Issue #15569

Summary of Changes

Fixed attribute checks.

Testing Instructions

Expected result

Actual result

Documentation Changes Required

@hikashop-nicolas

As mentioned here #15569 (comment) I have tested the change and confirm that it fixes the problem.

@PhocaCz

This comment has been minimized.

Show comment
Hide comment
@PhocaCz

PhocaCz Apr 26, 2017

Tested and working OK:

before the change:
Before change
after the change:
After change

But I get other issue regarding changes in calendar javascript between RC4 and stable:
#15574

PhocaCz commented Apr 26, 2017

Tested and working OK:

before the change:
Before change
after the change:
After change

But I get other issue regarding changes in calendar javascript between RC4 and stable:
#15574

@wilsonge wilsonge merged commit f461bd5 into joomla:staging Apr 26, 2017

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/drone/pr the build was successful
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@wilsonge wilsonge added this to the Joomla 3.7.1 milestone Apr 26, 2017

@wilsonge

This comment has been minimized.

Show comment
Hide comment
@wilsonge

wilsonge Apr 26, 2017

Contributor

Merging with two good tests. Thanks guys!

Contributor

wilsonge commented Apr 26, 2017

Merging with two good tests. Thanks guys!

@ggppdk

This comment has been minimized.

Show comment
Hide comment
@ggppdk

ggppdk Apr 26, 2017

Contributor

Yes, seems to be working properly for me too

Contributor

ggppdk commented Apr 26, 2017

Yes, seems to be working properly for me too

@dgrammatiko

This comment has been minimized.

Show comment
Hide comment
@dgrammatiko

dgrammatiko Apr 26, 2017

Member

@ggppdk @wilsonge @fevangelou unfortunately there is one more bug here:
these 2 lines https://github.com/joomla/joomla-cms/blob/staging/media/system/js/fields/calendar.js#L805-L806
needs to be moved before https://github.com/joomla/joomla-cms/blob/staging/media/system/js/fields/calendar.js#L804

Or we have:

		if (this.params.showsTodayBtn) {
			row = createElement("div", this.wrapper);
			row.className = "buttons-wrapper btn-group";

and should be:

		row = createElement("div", this.wrapper);
		row.className = "buttons-wrapper btn-group";
		
                if (this.params.showsTodayBtn) {

Will make PR tomorrow, unless someone else is faster

Member

dgrammatiko commented Apr 26, 2017

@ggppdk @wilsonge @fevangelou unfortunately there is one more bug here:
these 2 lines https://github.com/joomla/joomla-cms/blob/staging/media/system/js/fields/calendar.js#L805-L806
needs to be moved before https://github.com/joomla/joomla-cms/blob/staging/media/system/js/fields/calendar.js#L804

Or we have:

		if (this.params.showsTodayBtn) {
			row = createElement("div", this.wrapper);
			row.className = "buttons-wrapper btn-group";

and should be:

		row = createElement("div", this.wrapper);
		row.className = "buttons-wrapper btn-group";
		
                if (this.params.showsTodayBtn) {

Will make PR tomorrow, unless someone else is faster

rdeutz added a commit to joomlajenkins/joomla-cms that referenced this pull request May 1, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment