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

Fix subform.repeatable-table small screen padding #22335

Closed
wants to merge 1 commit into from

Conversation

ciar4n
Copy link
Contributor

@ciar4n ciar4n commented Sep 23, 2018

Pull Request for Issue #20210 .

Summary of Changes

Fix subform.repeatable-table small screen padding

Testing Instructions

Add the following after https://github.com/joomla/joomla-cms/blob/staging/modules/mod_login/mod_login.xml#L26

Open login module settings. Check repeatable field styling on reduced browser width.

<field name="sebform1" type="subform" label="subform"
	layout="joomla.form.field.subform.repeatable-table" multiple="true" groupByFieldset="true">
	<form>
		<fieldset name="group1" label="group 1">
			<field type="text" name="text" label="text 1"/>
			<field type="text" name="text2" label="text 2"/>
			<field type="text" name="text3" label="text 3"/>
		</fieldset>
		<fieldset name="group2" label="group 2">
			<field type="text" name="text4" label="text 4"/>
			<field type="textarea" name="textarea" label="textarea"/>
		</fieldset>
	</form>
</field>

Before

image

After

image

Documentation Changes Required

@Quy
Copy link
Contributor

Quy commented Sep 23, 2018

I have tested this item ✅ successfully on e3a6cba


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

@Harmageddon
Copy link
Contributor

I see the problem, but I'm not sure this PR solves it 100%. For example, look at this screenshot taken at a width of 1200px with this PR applied. Some of the inputs are already extremely narrow, and the table collides with the right column.
subform-repeatable-table

A simple solution for this would be to use the form-vertical class for this layout, that would lead to displaying the labels above the inputs instead of next to them.
Pro: Solves the problem of small inputs for all screen sizes. One might even consider dropping some of the media queries used right now.
Contra: Changes the look of this layout, leading to a small waste of space, but only on wide screens and in case the table has only one column.
subform-repeatable-table-form-vertical

@ciar4n
Copy link
Contributor Author

ciar4n commented Sep 28, 2018

@Harmageddon I agree. What you are suggesting makes far more sense. Do you wish to submit a PR and I can close this?

@Harmageddon
Copy link
Contributor

@ciar4n I just tested only adding form-vertical to the layout in staging, but then, the media query in _forms.less changes the layout again, so I'll have to take some time to look more into the meaning and history of this media query to avoid breaking anything. I probably won't have enough time during the weekend, but can do it on Monday. If you'd like to have this fixed before, feel free to do so. ;-)

@Harmageddon
Copy link
Contributor

@ciar4n Please have a look at #22444. It worked without changing the CSS now.

@ciar4n
Copy link
Contributor Author

ciar4n commented Oct 2, 2018

Closing this PR as it is replaced by #22444

@ciar4n ciar4n closed this Oct 2, 2018
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

4 participants