NPM Resource Changed ? Success

User tests: Successful: Unsuccessful:

avatar bahl24
bahl24
14 Feb 2019

Pull Request for Issue - When viewing information about an image the text overlaps when toggle menu is open both in PC as well as mobile devices.
Mobile
screenshot from 2019-02-14 16-41-25
PC
screenshot from 2019-02-14 16-41-08

Summary of Changes

Removed flex property which solves the issue both in PC and Mobile irrespective of whether toggle menu is open or closed
Mobile-
screenshot from 2019-02-14 16-40-11
PC-
screenshot from 2019-02-14 16-39-09

Testing Instructions

Components->media->select an image->info

Documentation Changes Required

No

Votes

# of Users Experiencing Issue
1/1
Average Importance Score
5.00

avatar bahl24 bahl24 - open - 14 Feb 2019
avatar bahl24 bahl24 - change - 14 Feb 2019
Status New ⇒ Pending
avatar joomla-cms-bot joomla-cms-bot - change - 14 Feb 2019
Category ⇒ Administration com_media
avatar bahl24
bahl24 - comment - 14 Feb 2019

@infograf768 @dgrammatiko @Quy @C-Lodder Kindly provide feedback regarding the design changes

avatar bahl24 bahl24 - change - 14 Feb 2019
The description was changed
avatar bahl24 bahl24 - edited - 14 Feb 2019
avatar C-Lodder
C-Lodder - comment - 15 Feb 2019

all seems fine to me

avatar joomla-cms-bot joomla-cms-bot - change - 21 Feb 2019
Category Administration com_media ⇒ Administration com_media NPM Change
avatar bahl24
bahl24 - comment - 21 Feb 2019

@infograf768 @Quy Kindly test this

avatar infograf768
infograf768 - comment - 21 Feb 2019

This does not solve RTL.
screen shot 2019-02-21 at 08 59 16

(Would need bootstrap override specific to RTL)

In my patch for RTL I have used ul/li and no flex to solve all issues, including in LTR.

RTL
screen shot 2019-02-21 at 09 18 16

LTR
screen shot 2019-02-21 at 09 18 48

Mobile
As you can see, I have also modified css to get more space for the name of the file in mobile vue

screen shot 2019-02-21 at 09 25 36

I have not proposed this patch yet as it waits for other PRs to be merged, specially
#23950
and
#23756

avatar bahl24
bahl24 - comment - 21 Feb 2019

Ok, I will close it then

On Thu 21 Feb, 2019, 2:04 PM infograf768 <notifications@github.com wrote:

This does not solve RTL.
[image: screen shot 2019-02-21 at 08 59 16]
https://user-images.githubusercontent.com/869724/53152871-0626bc80-35b7-11e9-8ba5-0b9d5f1d4236.png

(Would need bootstrap override specific to RTL)

In my patch for RTL I have used ul/li and no flex to solve all issues,
including in LTR.

RTL
[image: screen shot 2019-02-21 at 09 18 16]
https://user-images.githubusercontent.com/869724/53153934-cf9e7100-35b9-11e9-9fc4-1853a081015c.png

LTR
[image: screen shot 2019-02-21 at 09 18 48]
https://user-images.githubusercontent.com/869724/53153967-e04ee700-35b9-11e9-8aa4-2b2091f06099.png

Mobile
As you can see, I have also modified css to get more space for the name of
the file in mobile vue

[image: screen shot 2019-02-21 at 09 25 36]
https://user-images.githubusercontent.com/869724/53154275-b5b15e00-35ba-11e9-87c7-a5c03fadb066.png

I have not proposed this patch yet as it waits for other PRs to be merged,
specially
#23950 #23950
and
#23756 #23756

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#23902 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AibEyszO0DnZsKtlcqUYOAetex0sefOJks5vPln4gaJpZM4a7cT4
.

avatar infograf768
infograf768 - comment - 21 Feb 2019

Please don't close.
the decision to use ul/li instead of ol/etc. should be decided by maintainers.
@C-Lodder @wilsonge
I could update my future patch to your proposal with a bootstrap override and my other modififications should still be possible.

avatar C-Lodder
C-Lodder - comment - 21 Feb 2019

@infograf768 ol vs ul don't need a decision by maintainers. One is an ordered list (1, 2, 3, etc) and the other isn't. In general, using a list would probably be better as there shouldn't be any issues with RTL

avatar infograf768
infograf768 - comment - 21 Feb 2019

In general, using a list would probably be better as the RTL should kick in automatically.

I do prefer ul in general as they are easier to cope with and we do not need flex at all in this case.
Still needs some tweaks for RTL concerning com_media.

Look here the non-minified css file obtained after my patch. At the bottom of the file are all the RTL overrides, including for infovue.
mediamanager.min.css.zip

avatar infograf768 infograf768 - change - 26 Feb 2019
Labels Added: NPM Resource Changed
avatar infograf768
infograf768 - comment - 26 Feb 2019

OK, I am told that it is better to use <dl> etc.
Will try to deal with rtl, using your patch.

avatar infograf768 infograf768 - test_item - 26 Feb 2019 - Tested successfully
avatar infograf768
infograf768 - comment - 26 Feb 2019

I have tested this item ✅ successfully on 7cbfa13

OK for LTR. RTL is another story to be dealt with separately. Thanks.

One more tester please.


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

avatar infograf768
infograf768 - comment - 26 Feb 2019

I have tested this item ✅ successfully on 7cbfa13

OK for LTR. RTL is another story to be dealt with separately. Thanks.

One more tester please.


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

avatar infograf768
infograf768 - comment - 26 Feb 2019

@C-Lodder
Please confirm test.

avatar Razzo1987
Razzo1987 - comment - 26 Feb 2019

For me doesn't work :(

screen shot 2019-02-26 at 09 17 23


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

avatar infograf768
infograf768 - comment - 26 Feb 2019

@Razzo1987
Have you ran npm ci on the test site?
If not, then the patch is not enough...

avatar Razzo1987
Razzo1987 - comment - 26 Feb 2019

Sotty, I didn't.

I understand now I miss this step on my tests. Now I'm redone it


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

avatar wilsonge wilsonge - change - 26 Feb 2019
Status Pending ⇒ Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 ⇒ 2019-02-26 10:53:46
Closed_By ⇒ wilsonge
avatar wilsonge wilsonge - close - 26 Feb 2019
avatar wilsonge wilsonge - merge - 26 Feb 2019
avatar wilsonge
wilsonge - comment - 26 Feb 2019

Merged on JM's request. Thanks!

Add a Comment

Login with GitHub to post a comment