Defect #33339

Broken layout of the preview tab of "Welcome text" setting due to unexpectedly applied padding-left

Added by Go MAEDA 4 months ago. Updated 4 months ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Go MAEDA% Done:

0%

Category:Administration
Target version:4.1.2
Resolution:Fixed Affected version:4.0.7

Description

See the screenshots below. The preview tab has unwanted extra margin on the left side.

Edit:

Preview:

welcome-text-edit.png (29.6 KB) Go MAEDA, 2020-04-22 12:07

welcome-text-preview.png (21.3 KB) Go MAEDA, 2020-04-22 12:07

redminecrm_settings.png (175 KB) Alexander Meindl, 2020-04-26 15:22

sample-plugin.png (18.2 KB) Go MAEDA, 2020-04-26 15:30

helpdesk.png (117 KB) Alexander Meindl, 2020-04-26 17:12

settings_patch_with_plugin_classes.patch Magnifier (548 Bytes) Alexander Meindl, 2020-04-27 06:37


Related issues

Related to Redmine - Patch #33453: Add plugin CSS classes to plugin settings views Closed

Associated revisions

Revision 19720
Added by Go MAEDA 4 months ago

Broken layout of the preview tab of "Welcome text" setting due to unexpectedly applied padding-left (#33339).

Patch by Bernhard Rohloff.

Revision 19721
Added by Go MAEDA 4 months ago

Merged r19720 from trunk to 4.1-stable (#33339).

Revision 19729
Added by Go MAEDA 4 months ago

Fix UI issues in the setting page of some plugins that have arisen after r19720.

Patch by Marius BALTEANU.

Revision 19730
Added by Go MAEDA 4 months ago

Merged r19729 from trunk to 4.1-stable (#33339).

History

#1 Updated by Yuichi HARADA 4 months ago

The reason the preview looks like this is because the CSS style ".tabular.settings p{ padding-left: 300px; }" is working.

I think the following patch will solve the problem.

diff --git a/public/stylesheets/application.css b/public/stylesheets/application.css
index a7512bd48..26606f4bc 100644
--- a/public/stylesheets/application.css
+++ b/public/stylesheets/application.css
@@ -847,7 +847,7 @@ input#months { width: 46px; }
 .tabular.settings .wiki-preview, .tabular.settings .jstTabs { width: 99%; }
 .tabular .wiki-preview p {
   min-height: initial;
-  padding: 0;
+  padding: 0 !important;
   padding-top: 1em !important;
   padding-bottom: 1em !important;
   overflow: initial;

#2 Updated by Bernhard Rohloff 4 months ago

Yuichi HARADA wrote:

The reason the preview looks like this is because the CSS style ".tabular.settings p{ padding-left: 300px; }" is working.

I think the following patch will solve the problem.

[...]

As this padding can lead to further issues in other parts I would prefer to constrain it to only the direct children of the tabular settings container. I'm also not a huge fan of the !important statement and try to avoid it.

Index: public/stylesheets/application.css
===================================================================
--- public/stylesheets/application.css    (Revision 19718)
+++ public/stylesheets/application.css    (Arbeitskopie)
@@ -853,7 +853,7 @@
   overflow: initial;
 }

-.tabular.settings p{ padding-left: 300px; }
+.tabular.settings > p{ padding-left: 300px; }
 .tabular.settings label{ margin-left: -300px; width: 295px; }
 .tabular.settings textarea, .tabular.settings .wiki-preview, .tabular.settings .jstTabs { width: 99%; }

#3 Updated by Yuichi HARADA 4 months ago

Bernhard Rohloff wrote:

As this padding can lead to further issues in other parts I would prefer to constrain it to only the direct children of the tabular settings container. I'm also not a huge fan of the !important statement and try to avoid it.

[...]

Thanks for pointing out. I think your patch is better. I think too it's correct not to use "!important" whenever possible.

#4 Updated by Go MAEDA 4 months ago

  • Target version set to 4.1.2

Setting the target version to 4.1.2.

#5 Updated by Go MAEDA 4 months ago

  • Subject changed from The layout of the preview tab of "Welcome text" is broken to Broken layout of the preview tab of “Welcome text” setting due to unexpectedly applied padding-left
  • Status changed from New to Closed
  • Assignee set to Go MAEDA
  • Resolution set to Fixed

Committed the patch. Thank you for fixing the issue.

#6 Updated by Go MAEDA 4 months ago

  • Subject changed from Broken layout of the preview tab of “Welcome text” setting due to unexpectedly applied padding-left to Broken layout of the preview tab of "Welcome text" setting due to unexpectedly applied padding-left

#7 Updated by Alexander Meindl 4 months ago

Hi,

r19720 breaks alignment with lots of plugin settings, because this padding left is used in plugin.html.erb

If a plugin uses tabs for settings, this looks shifted to the left.

#8 Updated by Bernhard Rohloff 4 months ago

Alexander Meindl wrote:

Hi,

r19720 breaks alignment with lots of plugin settings, because this padding left is used in plugin.html.erb

If a plugin uses tabs for settings, this looks shifted to the left.

Do you have any example such a plugin, so we can look for a proper fix for the padding issues. The whole CSS styles for the padding inside these boxes look a bit chaotic to me.

#9 Updated by Alexander Meindl 4 months ago

Hi,

sure, e.g. redmineup CRM settings. See my attachment.

#10 Updated by Go MAEDA 4 months ago

  • Status changed from Closed to Reopened

#11 Updated by Go MAEDA 4 months ago

Alexander Meindl wrote:

sure, e.g. redmineup CRM settings. See my attachment.

To my eyes, it appears to be normal. And I could not find any problem with the sample plugin (/extra/sample_plugin). Could you describe more detail?

#12 Updated by Alexander Meindl 4 months ago

The problem only exist, if tabs are used in settings, e.g. with most of redmineup plugins.

#13 Updated by Go MAEDA 4 months ago

Alexander Meindl wrote:

The problem only exist, if tabs are used in settings, e.g. with most of redmineup plugins.

If the problem is specific to particular third-party plugins, it should be cared by the third-party vendor. Maybe the cause of the problem is the enhanced UI of the plugins. I recommend you to contact the vendor.

#14 Updated by Alexander Meindl 4 months ago

Are there any non third-party Plugins?

At the moment in template plugin.html.erb is set

<div class="box tabular settings">

The related css code is used by plugins. If css code changes in Redmine, all plugin developers have to take care of this change. But if you think this is the best solution, this should not be a problem.

#15 Updated by Go MAEDA 4 months ago

Alexander Meindl wrote:

The related css code is used by plugins. If css code changes in Redmine, all plugin developers have to take care of this change. But if you think this is the best solution, this should not be a problem.

I checked two plugins that I use (redmine_issue_templates and redmine-view-customize) and those plugins are not affected. I think the problem is specific to particular plugins that enhance the setting page.

And I could not find any problem in the screenshot . Could you describe what is the problem?

#16 Updated by Alexander Meindl 4 months ago

The labels of the settings are cut. There should be more space for it and moved to the right. On the picture the forth settings from top and the second last entry are not complete. Can you see it?

#17 Updated by Alexander Meindl 4 months ago

I attached another example screen with incorrect label lenght.

#18 Updated by Marius BALTEANU 4 months ago

Let's use for now this safer fix:

mariusbalteanu@Mariuss-MacBook-Pro redmine % git diff                          
diff --git a/public/stylesheets/application.css b/public/stylesheets/application.css
index 922be5eb9..330b1d486 100644
--- a/public/stylesheets/application.css
+++ b/public/stylesheets/application.css
@@ -847,6 +847,7 @@ input#months { width: 46px; }

 .tabular .wiki-preview, .tabular .jstTabs {width: 95%;}
 .tabular.settings .wiki-preview, .tabular.settings .jstTabs { width: 99%; }
+.tabular.settings .wiki-preview p {padding-left: 0 !important}
 .tabular .wiki-preview p {
   min-height: initial;
   padding: 0;
@@ -855,7 +856,7 @@ input#months { width: 46px; }
   overflow: initial;
 }

-.tabular.settings > p{ padding-left: 300px; }
+.tabular.settings p { padding-left: 300px; }
 .tabular.settings label{ margin-left: -300px; width: 295px; }
 .tabular.settings textarea, .tabular.settings .wiki-preview, .tabular.settings .jstTabs { width: 99%; }

#19 Updated by Go MAEDA 4 months ago

  • Assignee changed from Go MAEDA to Jean-Philippe Lang

I am not sure if it is the right way to change the code of Redmine in order to fix RedmineUP plugins.

I will leave the judgment to Jean-Philippe Lang.

#20 Updated by Alexander Meindl 4 months ago

Hi Go MAEDA,

I would suggest, that we stick to your solution. But it should be a possibility for plugin developers to change css for an individual namespace to the plugin settings (without using deface or Javascript).

If a css class would be added with the plugin name, a plugin developer can change css for the required plugin.

I added a patch for this possible change. It would be very helpful, also for other situations for a Redmine plugin developer.

#21 Updated by Go MAEDA 4 months ago

Alexander Meindl wrote:

I added a patch for this possible change. It would be very helpful, also for other situations for a Redmine plugin developer.

Thank you for sharing the patch. It would be nice if the patch will be a part of the next major release. Could you open a new issue for this feature?

#22 Updated by Marius BALTEANU 4 months ago

Go Maeda, why we don't revert the change committed and apply to safer fix which targets only the p element from settings wiki-preview?

#23 Updated by Bernhard Rohloff 4 months ago

Marius BALTEANU wrote:

Go Maeda, why we don't revert the change committed and apply to safer fix which targets only the p element from settings wiki-preview?

Since the current fix really seems to affect existing plugins I tend to go with Marius on this topic. It would be unfortunate to break existing plugins with a support release like v4.1.2.
The markup and CSS styles for forms are quite messy with multiple selectors, negative padding, no semantic structure. Perhaps it's better to clean this whole thing up in a future major release.

#24 Updated by Marius BALTEANU 4 months ago

Bernhard Rohloff wrote:

Marius BALTEANU wrote:

Go Maeda, why we don't revert the change committed and apply to safer fix which targets only the p element from settings wiki-preview?

Since the current fix really seems to affect existing plugins I tend to go with Marius on this topic. It would be unfortunate to break existing plugins with a support release like v4.1.2.
The markup and CSS styles for forms are quite messy with multiple selectors, negative padding, no semantic structure. Perhaps it's better to clean this whole thing up in a future major release.

Totally agree with your remarks.

#25 Updated by Go MAEDA 4 months ago

  • Assignee changed from Jean-Philippe Lang to Go MAEDA

Bernhard Rohloff wrote:

Since the current fix really seems to affect existing plugins I tend to go with Marius on this topic.

Do you know if there are any affected plugins other than RedmineUP's ones? If so, I will readily follow Marius' suggestion in #33339#note-18.

The reason I have been hesitating to commit the fix is that I am afraid of changing the code of Redmine just for fixing plugins from a particular third-party vendor. However, if plugins other than ones from RedmineUP are also affected, it's a different story.

#26 Updated by Mischa The Evil 4 months ago

Go MAEDA wrote:

[...]
The reason I have been hesitating to commit the fix is that I am afraid of changing the code of Redmine just for fixing plugins from a particular third-party vendor. However, if plugins other than ones from RedmineUP are also affected, it's a different story.

As far as I can see any plugin that has one or more text_area type setting and/or long settings labels will be affected like the RedmineUP example given by Alexander Meindl.

I think that the fix proposed by Marius in #note-18 is the proper one (for now).

#27 Updated by Go MAEDA 4 months ago

  • Status changed from Reopened to Closed

Committed the fix #33339#note-18.

Thank you all for your advice.

#28 Updated by Mischa The Evil 3 months ago

  • Related to Patch #33453: Add plugin CSS classes to plugin settings views added

Also available in: Atom PDF