Skip to content

Conversation

@pau1a
Copy link
Contributor

@pau1a pau1a commented Jul 18, 2019

Pull Request for Issue #25611 .

Summary of Changes

Added css to separate buttons as per Issue #25611. Currently unsure about any extant convention for positioning within file. Have tried to avoid the obvious elements that should be near the top of the file.

Testing Instructions

  1. Login to joomla admin panel
  2. Go to content -> media
  3. Click on create new folder

Expected result

fields should have some space

Actual result

fields should have some space

Documentation Changes Required

None

@C-Lodder
Copy link
Member

You need to update the LESS files and compile

@brianteeman
Copy link
Contributor

See this doc for instructions https://docs.joomla.org/Joomla_LESS/en

@pau1a
Copy link
Contributor Author

pau1a commented Jul 18, 2019

Ah I had my suspicions about that but couldn't be sure. Will amend.

@C-Lodder
Copy link
Member

Also, a general margin will add 10px of space in each direct. Instead I'd suggest adding a bottom margin to the parent container

@pau1a
Copy link
Contributor Author

pau1a commented Jul 18, 2019

How about a left/right margin of 2px as the buttons do seem to be right up against each other also.

@pau1a
Copy link
Contributor Author

pau1a commented Jul 18, 2019

Propose to place these changes into /administrator/templates/isis/less/blocks/_media.less

@gogicomputers
Copy link

I have tested this item ✅ successfully on 7073924


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

@ghost
Copy link

ghost commented Jul 20, 2019

@uthorat please test as opener of #25611.

How to: https://docs.joomla.org/Testing_Joomla!_patches

@N6REJ
Copy link
Contributor

N6REJ commented Jul 20, 2019

I would recommend instead of using 2px to use .25em instead. This is eqv to a "space" char, and being that its fluid will scale to other changes.

@brianteeman
Copy link
Contributor

It is fine as px. Everything else in this template is in px. Just changing this one value to em wont have the benefit you expect - you would need to convert everything

@SharkyKZ
Copy link
Contributor

SharkyKZ commented Jul 20, 2019

This breaks button group display. Margin should be applied to the form, not to the buttons. Maybe apply custom class to folder form, like .uploadform is applied to the upload form right above the folder form?

@pau1a
Copy link
Contributor Author

pau1a commented Jul 20, 2019

When you say button group do you mean the fact that the buttons are paired and touching like elsewhere in the backend for yes/no buttons etc?
If we agree that these buttons should be treated in the same way then Id think just removing the margin-right will get that job done..

@N6REJ
Copy link
Contributor

N6REJ commented Jul 20, 2019

@brianteeman actually thats not true, we specifically looked at both with the icons. They were changed from px to em ( specifically a single space ) which solved the issues. The point I think you missed is, .25em = char $32 which 2px does not.

@brianteeman
Copy link
Contributor

em is a relative measure
px is an absolute measure

Its not me that missed something at all

@N6REJ
Copy link
Contributor

N6REJ commented Jul 21, 2019

@brianteeman before you continue to spout on about what you clearly don't comprehend please go get educated. Your demeaning me on a public platform when you haven't even taken the time to familiarize yourself ACCURATELY with my statements.

As you pointed out a px is a finite measurEment COMPLETELY unrelated to the content and wholy related to the device. Ergo 2px on any given display may/may not be of different size.
em is a relative unit. The "space" character most often used on websites is roughly eqv to 2.5px. This is not possible for a ton of reasons so you must look to using some other form of measurement ( of which there are many )
turns out ( as we found out doing the icon fixing ) that .25em is the perfect size to emulate a "space" character.
Since this is a fluid display that we're working on we MUST move away from fixed sizing like was erroneously done decades ago.
With such a vast array of displays that we now have we have no clue what they will be used/viewed on. Ergo the push to responsive web design.
Your statement that "this is a px template" is prima facie true it is also false. It's a bootstrap based template which is a FLUID template! Because of convention it uses .px but its behavior's are primarily controlled by %'s & em's.
Lastly just because a method has been used in the past doesn't make it the best way.
But who am I to want to actually REMOVE a bug instead of trading one for another

edit: Just incase anyone doubts my statement he's the padding actually at work as it has been for a long time
image

@brianteeman
Copy link
Contributor

If writing that makes you happy then fine. It's factually and technically incorrect and based on many assumptions and misunderstandings, not least a failure to understand my original comment. I suggest you educate yourself better on what an em unit is

@N6REJ
Copy link
Contributor

N6REJ commented Jul 21, 2019

Let's see how well things work with px vs em...
( em )
image
(px)
image
Clearly there are issues.

@N6REJ
Copy link
Contributor

N6REJ commented Jul 21, 2019

@pau1a while your fixing the spacing would you please fix the .29px shift between field and button, and the .29px padding offset in button text.
image

@brianteeman
Copy link
Contributor

Your calculation to convert px to em is not correct which is why you see the difference

image

@N6REJ
Copy link
Contributor

N6REJ commented Jul 21, 2019

I never said 2px = .25em, I said .25em = chr$32! .25em is closer to 3px then 2px lol

@ghost
Copy link

ghost commented Jul 26, 2019

I have tested this item ✅ successfully on dccdd41


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

@ghost
Copy link

ghost commented Jul 26, 2019

@gogicomputers can you please retest?

@gogicomputers
Copy link

I have tested this item ✅ successfully on 2e041f3


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

@ghost
Copy link

ghost commented Jul 27, 2019

@infograf768 does this need a second retest?

@infograf768
Copy link
Member

not necessary. can be set rtc

@ghost
Copy link

ghost commented Jul 27, 2019

Status "Ready To Commit".

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 27, 2019
@HLeithner
Copy link
Member

Thank you for making Joomla! more pretty.

@HLeithner HLeithner merged commit 6093107 into joomla:staging Aug 8, 2019
@HLeithner HLeithner added this to the Joomla! 3.9.11 milestone Aug 8, 2019
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Aug 8, 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.

10 participants