? ? Failure

User tests: Successful: Unsuccessful:

avatar dgrammatiko
dgrammatiko
13 Dec 2016

Pull Request for Issue #13166.

Summary of Changes

  • Add one more class
  • Remove some useless padding in the css
  • Drop the vanilla word from the filenames (css and js)

Testing Instructions

Apply the test and try the calendar (e.g. article edit

Preview:

LTR
screen shot 2016-12-14 at 16 49 47
screen shot 2016-12-14 at 16 50 10

RTL
screen shot 2016-12-14 at 16 51 10
screen shot 2016-12-14 at 16 51 28

KUDOS to @ciar4n for cleaning up the css!! Thank you!!

Documentation Changes Required


This change is Reviewable

avatar dgt41 dgt41 - open - 13 Dec 2016
avatar dgt41 dgt41 - change - 13 Dec 2016
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 13 Dec 2016
Category Layout JavaScript
avatar dgt41 dgt41 - change - 13 Dec 2016
The description was changed
avatar dgt41 dgt41 - edited - 13 Dec 2016
avatar dgt41 dgt41 - change - 13 Dec 2016
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - change - 13 Dec 2016
Category Layout JavaScript Layout JavaScript Unit Tests
avatar andrepereiradasilva
andrepereiradasilva - comment - 13 Dec 2016

@dgt41 didn't test this, but can't we get a better color than this lime fluorescent green ... ?

avatar dgt41
dgt41 - comment - 13 Dec 2016

Sure, any suggestion?

avatar andrepereiradasilva
andrepereiradasilva - comment - 13 Dec 2016

will not make any proposal, desing is really not by strong ... just think this lime is not good because flurescent colors on screens are very bright
Maybe @ciar4n can do a color proposal ?

avatar ciar4n
ciar4n - comment - 13 Dec 2016

@andrepereiradasilva I'll do a general style clean up once @dgt41 is happy with the rest! ?

@dgt41 Did you have a chance to look at the colspan count in the time tr when there is no 'wk' column?..

<tr class="time">
	<td class="time time-title" colspan="1">Time:</td>
	<td class="time hours-select" colspan="2"> .. </td>
	<td class="time minutes-select" colspan="2"> .. </td>
	<td class="time ampm-select" colspan="3"> </td>
</tr>

All colspan should total 7 not 8. This would allow us to fix the un-even column widths.

avatar dgt41
dgt41 - comment - 13 Dec 2016

@ciar4n the colspan is fixed! You can take over here

avatar ciar4n
ciar4n - comment - 13 Dec 2016

@dgt41 Cool.. should get a chance to look at this tomorrow.

avatar ciar4n
ciar4n - comment - 14 Dec 2016

@dgt41 Could you checkout dgt41#46

calendar1

avatar dgt41 dgt41 - change - 14 Dec 2016
Labels Added: ?
avatar dgt41 dgt41 - edited - 14 Dec 2016
avatar schnuti
schnuti - comment - 15 Dec 2016

Did I miss something? I installed staging from 2016-12-12 + patch 13153 and this 13181.
Result is that the top of the pop-up is missing. Screen size 1366x768 - Firefox.
created-date
start-publishing

avatar ciar4n
ciar4n - comment - 15 Dec 2016

@schnuti Nothing missed.. you have highlighted a potential issue.

The calendar displays above the field if there is not enough space in the viewport to display below it...

3cb30b6b25f8cd6e8ebf56b7f92e1b96

@dgt41 Would it be possible to disable this in the JS? Appears to be adding a minus margin-top if the space below is not available. If not you could override it with CSS...

.js-calendar {
    margin-top: 0 !important;
}
avatar dgt41
dgt41 - comment - 15 Dec 2016

@ciar4n I can revert that part or even better improve it so that the calendar actually moves within the given viewport (?)

The code right now is just flipping the calendar from below (standard) to above the field

avatar infograf768
infograf768 - comment - 16 Dec 2016

Here the display above happens when one has chnaged the window size and did not reload the page. After reload, the calendar displays normally.

avatar schnuti
schnuti - comment - 16 Dec 2016

My screenshots above come from my "normal" full screen Firefox. (Bootstrap is not collapsed) I now tested to enlarge the viewport. After clicking CTRL + - three (3) times and page reload, the pop-ups appears in full below the fields.

In frontend (Protostar as is) "Create Article" shows the "Start Publishing" above and "Finish Publishing" below the field. "Finish Publishing" enlarges the scrollable area. A trick is, that you can't scroll by clicking the scrollbar. By using the mouse-wheel I can scroll down (or up) to see the pop-up in full.

avatar ciar4n
ciar4n - comment - 16 Dec 2016

@dgt41 Personally I wouldn't be a fan of moving the calendar with the viewport. My reasoning been that if a user decides to scroll while the calendar is opened, one can only presume that the calendar is no longer their focus.

A solution to the issue would be to only display the calendar above the field if the space is not available below within the entire page rather than just the viewport.

avatar PhilETaylor
PhilETaylor - comment - 22 Dec 2016

I have tested this item successfully on deacf6f

This fixes the issue I reported in #13166

Just one note, when I click "Today" I would assume I would be taken to today, highlighted, and the calendar stay open. It closes. This is against my expectation, dunno if others think that way though.


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

avatar PhilETaylor PhilETaylor - test_item - 22 Dec 2016 - Tested successfully
avatar anibalsanchez
anibalsanchez - comment - 1 Jan 2017

I have tested this item successfully on deacf6f

Test OK


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

avatar anibalsanchez anibalsanchez - test_item - 1 Jan 2017 - Tested successfully
avatar jeckodevelopment
jeckodevelopment - comment - 1 Jan 2017

@dgt41 can you please look at the conflicts?

avatar dgt41
dgt41 - comment - 1 Jan 2017
avatar genesisfan
genesisfan - comment - 15 Jan 2017

I have tested this item successfully on 6c78362


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

avatar genesisfan genesisfan - test_item - 15 Jan 2017 - Tested successfully
avatar jeckodevelopment
jeckodevelopment - comment - 15 Jan 2017

again conflicts Dimitris!

avatar bertmert
bertmert - comment - 17 Jan 2017

I have tested this item ? unsuccessfully on 6c78362

I cannot set year of Created Date. Zoom is 100%.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13181.
avatar bertmert bertmert - test_item - 17 Jan 2017 - Tested unsuccessfully
1003396 18 Jan 2017 avatar dgrammatiko fixes
7e4873d 18 Jan 2017 avatar dgrammatiko tests
avatar laoneo
laoneo - comment - 3 Feb 2017

@bertmer, I can't reproduce the issue. Did you try it with the latest staging?

image

avatar ciar4n
ciar4n - comment - 3 Feb 2017

If there is not enough space in the viewport below the field, the calendar position flips to above it.

@dgt41 I think disabling this is the way to go. At least then the calendar will always be accessible even thou in some cases the user may have to scroll.

avatar dgt41
dgt41 - comment - 3 Feb 2017

@ciar4n we discussed this in London, so shall I change the position to relative and the calendar will expand below the input?

avatar ciar4n
ciar4n - comment - 3 Feb 2017

Only issue there is your effecting the remaining content of the page which may cause some random issues depending on the content following the calendar field.

avatar dgt41
dgt41 - comment - 3 Feb 2017

@ciar4n ehm, having the input field always visible narrows down the possible solutions

avatar ciar4n
ciar4n - comment - 3 Feb 2017

@dgt41 Agreed

That considered, I feel simply disabling the flip is the best option so the calendar always displays below. It is expected behavior and would resolve the issue of the calendar ever been inaccessible outside of the viewport.

avatar schnuti
schnuti - comment - 3 Feb 2017

Quote

I think disabling this is the way to go. At least then the calendar will always be accessible even thou in some cases the user may have to scroll.

Be still aware that clicking the scrollbar closes the pop-up and any expanded part of the viewport. i.e. If there is not enough space in the viewport below the field the user has to use alternative scrolling methods. If available - mouse wheel or ...?

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 4 Feb 2017

I have tested this item successfully on fb315d9


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

avatar franz-wohlkoenig franz-wohlkoenig - test_item - 4 Feb 2017 - Tested successfully
avatar rmittl
rmittl - comment - 5 Feb 2017

I have tested successfully in different browser and size windows.

avatar widmann-it
widmann-it - comment - 5 Feb 2017

I have tested this item successfully on fb315d9

We have testet succesfully on Firefox and Chrome on Windows and Linux
testet by jc17de


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

avatar widmann-it widmann-it - test_item - 5 Feb 2017 - Tested successfully
avatar zero-24 zero-24 - change - 5 Feb 2017
Milestone Added:
Status Pending Ready to Commit
Labels Added: ?
avatar wilsonge wilsonge - change - 5 Feb 2017
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2017-02-05 23:25:40
Closed_By wilsonge
Labels
avatar wilsonge wilsonge - close - 5 Feb 2017
avatar wilsonge wilsonge - merge - 5 Feb 2017
avatar csthomas
csthomas - comment - 6 Feb 2017

Please fix travis error.

  1. JHtmlTest::testCalendar

Line:1509 JS file "calendar-vanilla.min.js" should be loaded
Failed asserting that an array has the key '/media/system/js/fields/calendar-vanilla.min.js'.
/home/travis/build/joomla/joomla-cms/tests/unit/suites/libraries/cms/html/JHtmlTest.php:1510

avatar rdeutz
rdeutz - comment - 6 Feb 2017

I fixed the test

avatar csthomas
csthomas - comment - 6 Feb 2017

Great

avatar dgt41
dgt41 - comment - 6 Feb 2017

Thanks @rdeutz, somehow I messed one of the rebases

Add a Comment

Login with GitHub to post a comment