Meta Issue:#1870944: [Meta] Mobile friendly admin pages
Problem/Motivation
- Alignment of text in action dropdown. In Narrow screens the text is partly hidden.
- dropbutton cuts off long action item, either in the button or at the edge of the window.
Reproduce:
- Apply bug.patch
- Go to admin/structure/types/manage/article/fields
- Trying to reduce the width of browser window
Proposed resolution
A. Comment #57: Trying to fix both issues with a JS solutions. When the buttons placed nearly at edge of the windows side. Dropdown menu expands in same positions. see image in Comment #57. Table columns retains as the smallest as possible.
B. Comment #63: The column width will change according to the longest widget. BUT when it expended, the longest menu still cut off.
C. Comment #97: Patch flying in with a pure css solution that unifies the dropbutton width to that of the longest item.
Remaining tasks
- Needs a CSS approach to resolve both problem and retains performance.
- If above is failing, reviews the performance affection of Comment #57 patch.
Comment | File | Size | Author |
---|---|---|---|
#167 | drupal-core-1890266.gif | 3.83 MB | VladimirAus |
#160 | interdiff_156-160.txt | 1.58 KB | komalk |
#160 | 1890266-160.patch | 5.73 KB | komalk |
#158 | afterPatch#156.png | 23.61 KB | Ruchi Joshi |
#156 | interdiff_155-156.txt | 1.54 KB | komalk |
Comments
Comment #1
LewisNyman CreditAttribution: LewisNyman commentedNovice tags
Comment #2
edrupal CreditAttribution: edrupal commentedAny thoughts as to what would be a good solution.
I'd be happy to take the task on but not sure of the best way to move forward
Comment #3
hansrossel CreditAttribution: hansrossel commentedAttached patch does not use position:absolute in mobile. I tested also other places where the dropwidget is used, like admin/structure/menu
Comment #4
dawehnerSomething which really helps to review such issues are two screenshots showing before and after the patch.
Comment #5
edrupal CreditAttribution: edrupal commented1. Before screenshot of url /admin/config/content/formats
2. After screenshot of url /admin/config/content/formats
Comment #6
edrupal CreditAttribution: edrupal commentedComment #7
echoz CreditAttribution: echoz commentedNote that #3 patch, removing position: absolute; stretches the buttons wide, and the plain button that is not a drop-button is equally as wide. Screenshots attached.
Comment #8
edrupal CreditAttribution: edrupal commentedDid a bit more testing. In addition to the point raised in #7, the patch from #3 causes a change in the centring of the text in other columns when the drop down is expanded.
1. Before screenshot of url /admin/config/content/formats - drop down expanded
2. After screenshot of url /admin/config/content/formats - drop down expanded
Comment #9
edrupal CreditAttribution: edrupal commentedIgnore #9 - finger trouble
Comment #10
edrupal CreditAttribution: edrupal commentedAnother approach would be to use javascript to set the minimum width of the dropbutton-wrapper to that of the widest dropbutton. This patch attempts to implement this approach.
Comment #11
hansrossel CreditAttribution: hansrossel commentedabout the issues in #7, you are right, but those would be solved if the media query of patch #3 is set to 400 instead of 901. I need to measure more exactly at which width the issue starts and use that value in the media query. I cannot make a patch at the moment. On the other hand the configure text might have a different length in other languages so the js approach of the latest patch is then safer, I will test when I find wifi.
Comment #12
echoz CreditAttribution: echoz commented@10 patch, awesome! I hope you're on to the solution, but I found that when I click disable and cancel, or just click configure of one of the drop buttons, then go back to the previous screen (add text format) the js fix is no longer being used, the buttons are squished again, until I clear the site's cache, reload doesn't do it.
Comment #13
echoz CreditAttribution: echoz commentedComment #14
echoz CreditAttribution: echoz commentedComment #15
edrupal CreditAttribution: edrupal commentedOn further investigation to get the correct button width the button must be positioned relative. Additionally, before calculating the width the button wrapper must be floated so it doesn't take the column width when the screen is wide.
This version of the patch takes the following approach:
1. Initially set the button wrapper and widget to floated left and position relative, respectively.
2. Use js to calculate the widest button widget.
3. Set the width of all the button widgets to the widest one.
4. Use js to reset the button wrapper and widget styles so they display correctly.
Would be good to get feedback on whether this is a valid approach.
Comment #16
edrupal CreditAttribution: edrupal commentedComment #17
edrupal CreditAttribution: edrupal commentedUpdate to the comments to better fit in with drupal standards.
Comment #18
YesCT CreditAttribution: YesCT commented#17: dropbutton-text-width-1890266-17.patch queued for re-testing.
Comment #20
edrupal CreditAttribution: edrupal commentedRe-rolled patch in case that's why it's failing
Comment #21
edrupal CreditAttribution: edrupal commentedComment #22
star-szrThanks for your work on this everyone, especially @edrupal. Coding standards/documentation review follows:
Can this be reworded? "the wider of button or $widestButton" sounds odd. Maybe "Compares button and $widestButton and returns the largest value."? See also point #6.
s/buttom/button. These comments also need to end in a period per http://drupal.org/node/1354#drupal.
String is redundant, how about removing "A string representing”, so it’s “The width to compare…”?
Again, string is redundant – maybe just “The largest width value."?
These lines need to be indented by two spaces instead of one.
Is there a reason why the second parameter has a $ and the first doesn't?
Comment #23
edrupal CreditAttribution: edrupal commentedThanks for the excellent feedback @Cottser. I've incorporated all the comments into this latest patch.
Comment #24
tim.plunkettWhy are we using class names that map 1:1 with CSS properties? I don't know of anywhere else in core that is done, or why it is appropriate here.
Comment #25
star-szrThanks for the quick turnaround, @edrupal! Agreed with @tim.plunkett, those class names could be improved, and maybe combined into one class name - something like "processed"? Not sure what the convention is there.
Some more documentation/standards suggestions and notes:
Wording on this could be improved (parts are a bit terse), and this should be an inline comment (//). Something like "Set the minimum width of the first dropbutton wrapper to that of the widest dropbutton to prevent button text overflow on small screens."
This should be an inline comment as well. This comment confused me at first, here's another suggestion but could be improved by someone who understands this patch more than I do: "In order to accurately calculate the button width, the button wrapper is initially floated and the widget is positioned relatively."
On the next line, can we replace "settings must be undone" with "attributes must be overridden" or something along those lines? CSS doesn't have settings :)
This is now too long, the summary line must be under 80 characters per http://drupal.org/node/1354#drupal.
Sorry for the confusion - I meant the "A string…" text from the parameter description was redundant. The {String} data types should be kept.
Comment #26
star-szrOops, this was missed. Should be "$button" :)
Comment #27
edrupal CreditAttribution: edrupal commentedThanks for your patience and feedback. Here's another version.
Comment #28
edrupal CreditAttribution: edrupal commentedTypo correction and change of name of the 'processed' class.
Comment #29
tim.plunkettTagging.
Comment #30
star-szrThanks @edrupal, looks good to me. Only a couple more tiny nits to pick, leaving at needs review though:
Extra line here after the @return description and before the
*/
is not needed.html should be capitalized.
Edit: Added more context to @return code snippet.
Comment #30.0
star-szradded drupal/admin/structure/taxonomy
Comment #31
YesCT CreditAttribution: YesCT commentedupdated issue summary remaining tasks section.
Comment #32
edrupal CreditAttribution: edrupal commentedThanks. Updated patch and another slight change to name of 'processed' class.
Comment #33
YesCT CreditAttribution: YesCT commentedthese changes to the code look good.
I noticed some of the lines could have been wrapper longer but still under 80 chars.
Also, there was a block of comments missing one space indentation.
I made those changes.
next steps here: manual testing (see contributor task doc on manual testing: http://drupal.org/node/1489010)
Comment #34
YesCT CreditAttribution: YesCT commentedat first this looked really good.
before patch: cuts off
after patch:
no cut off
then I noticed that the row height changed when the button expanded making the other rows jump and move.
this does not happen without the patch.
I was looking for a button that had a short word on top, and a longer word under.
with translation enabled, the content list has that.
I noticed that the column widths change when the button is expanded.
this does not happen without the patch.
compare this:
with this:
with no patch:
with no patch: goes over edge.
I tried to apply the patch and get the row height to change again...
now I cannot reproduce the row height changing and I'm confused. I cleared caches on the site, and the browser cache... I'm not sure why.
Posting what I have now. Me or someone else can come back and do another round of manual testing.
Q1: Do we want the row height to change when the drop button is expanded? (I think we do not want that.)
Q2: Is column width changing when the drop button is expanded a blocker to this? (I think yes.)
In general, I think we dont want the page to jump around.
The approach in the patch is to set the width of the first item in the button to be the width of the widest button.
Comment #35
star-szrThe @params need some touch-up, see these comments on another issue (#77, #81, #83, #85):
#1847084-77: Measure/track displacing elements better + provide change events for them (fix overlay + toolbar)
#1847084-81: Measure/track displacing elements better + provide change events for them (fix overlay + toolbar)
#1847084-83: Measure/track displacing elements better + provide change events for them (fix overlay + toolbar)
#1847084-85: Measure/track displacing elements better + provide change events for them (fix overlay + toolbar)
In short: Remove the curly braces, remove the blank lines between each @param, and I would think Jquery needs to be jQuery.
Comment #36
edrupal CreditAttribution: edrupal commentedHi @Cottser, would it be best practice to change the @param blocks in all the other functions in the program dropbutton.js so there is consistency, even if they are not related to this issue?
Comment #37
star-szr@edrupal - Good question. We should only change the ones in this patch, and we'll file followup issue(s) to correct the existing curly braces and extra lines.
Comment #38
star-szrPlease see #1958246-2: Update JS documentation to follow Doxygen format (remove curly braces) - in addition to #35 it looks like button should be $button and widestButton should be widestButton. JavaScript docs are complicated! :)
Comment #39
edrupal CreditAttribution: edrupal commentedAs well as the issues already identified in #34 the previous patch also caused problems on the views admin pages where the hidden buttons in the advanced section where not sized correctly.
I've re-written the patch. The principle has not changed - to calculate the width of the widest text in the button and set the button width to this value.
Now the change is restricted to only work on buttons that are in the column of a table, and makes sure there are no hidden button elements before calculating its width.
This change is to ensure buttons that are not part of a column table are unaffected (which is where all the known issues occur).
1a. Content list - before patch applied (/admin/content)
1b. Content list - after patch applied (/admin/content)
2a. Text format configuration - button closed - before patch applied (/admin/config/content/formats)
2b. Text format configuration - button closed - after patch applied (/admin/config/content/formats)
3a. Text format configuration - button open - before patch applied (/admin/config/content/formats)
3b. Text format configuration - button open - after patch applied (/admin/config/content/formats)
Comment #41
edrupal CreditAttribution: edrupal commented#39: dropbutton-text-width-1890266-39.patch queued for re-testing.
Comment #42
tstoecklerRe @YesCT:
I tend to agree with you but these two details were specifically implemented the way they are in the original implementation. So if that bugs you I think you have to open a new issue and fight Bojhan or yoroy :-)
Comment #43
echoz CreditAttribution: echoz commented#42, the way the dropbuttons currently work does not expand row height or width when expanded, because of the absolute positioning.
Comment #44
YesCT CreditAttribution: YesCT commentedI like the functionality of this patch a lot.
No jumping around of the table rows. :)
Removing needs manual testing tag. (I did not check views, but looks like @edrupal did)
I'm not sure about the need for $ or no $ as @Cottser mentioned in #38, so a standards check of js would be nice.
Comment #45
edrupal CreditAttribution: edrupal commentedMinor update to correct the variable names to fit in with standards.
Comment #46
tim.plunkettThese should also probably be assigned into a variable first
In both places here we do
$('.dropbutton-widget', $dropbuttons[i]);
, why not assign that to a variable first?Comment #47
nod_I'm really not comfortable with the JS that's added here.
Before the patch, 1 dropbutton took 1ms to initialize. After the patch: 5ms. After the patch and some work on it: 4ms.
There have been lots of work to make the footprint of dropbutton JS as low as possible, this totally defeats the purpose. Think of a view with 50 dropbuttons on a mobile phone (like the admin/content page for example, it'll be a view at some point). Rendering takes 50ms. With the patch it takes 250ms. On a mobile? it takes 500ms and with the patch it would take 2.5 seconds, on top of all jQuery, Backbone and whatever else is on the page. This can not work.
Also we can't be using the -processed class in our CSS. It comes from the jQuery.once plugin and that plugin will soon be using data-attributes that you shouldn't be using in CSS to begin with.
I'm not sure of the way forward but it's not a JS-heavy one. Try to see if a CSS-only solution will work out please. you'll probably have to mess around with the current styling.
I can't support a patch that will make admin/content page take 2.5s to render mobile device because of dropbuttons.
Comment #48
hiddenfellon CreditAttribution: hiddenfellon commentedIf you're concerned about the js then another solution would be just set the min-width to 100px, I don't know you guys feel about that approach.
Comment #49
YesCT CreditAttribution: YesCT commented@nod_ 's comment on performance is pretty important.
related:
#1849750: Minimize all "Operations" columns width
#1914800: Dropbutton width is smaller than longest item
updating issue summary.
Comment #49.0
YesCT CreditAttribution: YesCT commentedupdated remaining tasks
Comment #50
RainbowArrayIt seems like the fundamental problem here is that some text strings may be too long for the space available, particularly on vertical mobile viewports.
This is probably beyond the scope of this particular issue, but it seems to me that a responsive tables solution might be a good way to go here.
Here's one of my favorite methods: http://css-tricks.com/examples/ResponsiveTables/responsive.php
Essentially this flips the table from horizontal to vertical. The column headers become row headers, with each of the previous rows as a new section on the table. There is a lot more vertical scrolling, but the upside is that it should be much better performance wise than getting JS involved. And this should be able to handle text strings of arbitrary length in a better way than before.
Again, maybe too radical a change. Just a thought of another way to go.
Comment #51
klonos...this issue needs to either be switched to a bug or its title changed to something like "Make dropbutton text retain .dropbutton-widget width." in order to reflect it being a task. Just saying...
Comment #52
echoz CreditAttribution: echoz commentedChanging to "bug report" and component: "Seven theme", but adding tag "JavaScript" for monitoring in case solutions still involve js.
Comment #52.0
echoz CreditAttribution: echoz commentedadded related issues and updated proposed resolution and remaining tasks.
Comment #53
jaffaralia CreditAttribution: jaffaralia commentedThe patch #45 did not apply. it shows this error: core/misc/dropbutton/dropbutton.base.css: No such file or directory
Comment #54
philipz CreditAttribution: philipz commentedI think this issue is related to #1989470: Dropbutton style update for Seven where this problem is fixed as far as I remember.
Comment #55
droplet CreditAttribution: droplet commentedComment #56
klonosComment #57
droplet CreditAttribution: droplet commentedThe above patches expand the TR column as long as possible (same width as the longest dropbutton menu). Personally I prefer to expand the menu over the element directly and do not modify the TR width.
Attached a patch to demonstrate my thoughts.
Comment #58
YesCT CreditAttribution: YesCT commentedComment #59
YesCT CreditAttribution: YesCT commentedapplies, removing needs reroll tag.
Comment #60
YesCT CreditAttribution: YesCT commenteddoes not apply anymore.
tagging needs reroll and adding instructions in the issue summary remaining tasks section.
Comment #61
LewisNyman CreditAttribution: LewisNyman commentedComment #62
Jalandhar CreditAttribution: Jalandhar commentedAs per #60
Comment #63
Risse CreditAttribution: Risse commentedMy proposition: make the dropbutton-widget right aligned to look more consistent. The column width will change according to the longest widget.
Comment #64
Risse CreditAttribution: Risse commentedComment #65
Anonymous (not verified) CreditAttribution: Anonymous commentedComment #66
ravi.khetri CreditAttribution: ravi.khetri commentedRe-rolled.
Comment #67
ravi.khetri CreditAttribution: ravi.khetri commentedRe-rolled.
Comment #68
YesCT CreditAttribution: YesCT commentedremoving the needs reroll tag, since it applies.
Needs before and after screenshots added to the summary
and a
review.
Comment #69
no_angel CreditAttribution: no_angel commentedI am not able to reproduce the problem on a fresh download of D8 (see attached before)
Please advise if there is something I need to do.
Comment #70
no_angel CreditAttribution: no_angel commentedComment #71
no_angel CreditAttribution: no_angel commented1989470 Related
changed status to closed.
Comment #72
droplet CreditAttribution: droplet commentedPlease don't close it randomly. We still discuss for a right approach
Comment #73
LewisNyman CreditAttribution: LewisNyman commentedI think no_angel couldn't produce the problem, can we confirm this is still a problem?
Comment #74
droplet CreditAttribution: droplet commented@nod_ #47 worrying about the JS performance but Patch in Comment #57 is a difference approach that won't add extra JS execution time to Page Loading.
Comment #75
no_angel CreditAttribution: no_angel commentedI worked on this again and am able to reproduce the issue with the bug.patch applied.
no_angel:drupal admin$ git apply bug.patch
no_angel:drupal admin$ ls -al
Firefox crashed when I minimized the window Figure 1, after bug.patch
Applied patch 1890266-66_0.patch
no_angel:drupal admin$ git apply 1890266-66_0.patch and cleared cache
(use "git add ..." to include in what will be committed)
1890266-66_0.patch
bug.patch
sites/default/files/
sites/default/services.yml
sites/default/settings.php
no changes added to commit (use "git add" and/or "git commit -a")
no_angel:drupal admin$ git diff
diff --git a/core/misc/dropbutton/dropbutton.css b/core/misc/dropbutton/dropbutt
index f089874..c4b5b61 100644
--- a/core/misc/dropbutton/dropbutton.css
+++ b/core/misc/dropbutton/dropbutton.css
@@ -33,16 +33,12 @@
.js .form-actions .dropbutton-widget {
position: static;
}
-.js td .dropbutton-widget {
- position: absolute;
-}
.js td .dropbutton-wrapper {
min-height: 2em;
}
.js td .dropbutton-multiple {
- padding-right: 10em; /* LTR */
- margin-right: 2em; /* LTR */
max-width: 100%;
+ float: right;
}
[dir="rtl"].js td .dropbutton-multiple {
padding-right: 0;
diff --git a/core/modules/field_ui/src/FieldConfigListBuilder.php b/core/modules
index 167d9f7..b347cf0 100644
--- a/core/modules/field_ui/src/FieldConfigListBuilder.php
+++ b/core/modules/field_ui/src/FieldConfigListBuilder.php
@@ -166,7 +166,7 @@ public function getDefaultOperations(EntityInterface $entity
if ($entity->access('update') && $entity->hasLinkTemplate("{$entity->entity
$operations['edit'] = array(
- 'title' => $this->t('Edit'),
+ 'title' => $this->t('Edit ettings settings settings'),
'weight' => 10,
'url' => $entity->urlInfo("{$entity->entity_type}-field-edit-form"),
);
@@ -180,7 +180,7 @@ public function getDefaultOperations(EntityInterface $entity
}
$operations['storage-settings'] = array(
- 'title' => $this->t('Storage settings'),
+ 'title' => $this->t('Storage settings settings settings settings settings
'weight' => 20,
'attributes' => array('title' => $this->t('Edit storage settings.')),
'url' => $entity->urlInfo("{$entity->entity_type}-storage-edit-form"),
...skipping...
diff --git a/core/misc/dropbutton/dropbutton.css b/core/misc/dropbutton/dropbutt
index f089874..c4b5b61 100644
--- a/core/misc/dropbutton/dropbutton.css
+++ b/core/misc/dropbutton/dropbutton.css
@@ -33,16 +33,12 @@
.js .form-actions .dropbutton-widget {
position: static;
}
-.js td .dropbutton-widget {
- position: absolute;
-}
.js td .dropbutton-wrapper {
min-height: 2em;
}
.js td .dropbutton-multiple {
- padding-right: 10em; /* LTR */
- margin-right: 2em; /* LTR */
max-width: 100%;
+ float: right;
}
[dir="rtl"].js td .dropbutton-multiple {
padding-right: 0;
diff --git a/core/modules/field_ui/src/FieldConfigListBuilder.php b/core/modules
index 167d9f7..b347cf0 100644
--- a/core/modules/field_ui/src/FieldConfigListBuilder.php
+++ b/core/modules/field_ui/src/FieldConfigListBuilder.php
@@ -166,7 +166,7 @@ public function getDefaultOperations(EntityInterface $entity
if ($entity->access('update') && $entity->hasLinkTemplate("{$entity->entity
$operations['edit'] = array(
- 'title' => $this->t('Edit'),
+ 'title' => $this->t('Edit ettings settings settings'),
'weight' => 10,
'url' => $entity->urlInfo("{$entity->entity_type}-field-edit-form"),
);
@@ -180,7 +180,7 @@ public function getDefaultOperations(EntityInterface $entity
}
$operations['storage-settings'] = array(
- 'title' => $this->t('Storage settings'),
+ 'title' => $this->t('Storage settings settings settings settings settings
'weight' => 20,
'attributes' => array('title' => $this->t('Edit storage settings.')),
'url' => $entity->urlInfo("{$entity->entity_type}-storage-edit-form"),
~
(END)
Comment #76
no_angel CreditAttribution: no_angel commentedremoved Amsterdam2104 tag.
Comment #77
no_angel CreditAttribution: no_angel commentedComment #78
droplet CreditAttribution: droplet commentedThanks @no_angel but I remove your summary updates. That would confuse other developers.
Comment #79
droplet CreditAttribution: droplet commentedComment #80
droplet CreditAttribution: droplet commentedComment #81
geek-merlinPatch flying in with a pure css solution that unifies the dropbutton width to that of the longest item.
Comment #82
geek-merlinThis one is the correct one.
Comment #84
star-szrTest failures in:
ExpandDrupal\config\Tests\ConfigFormOverrideTest
ExpandDrupal\system\Tests\DrupalKernel\ContainerRebuildWebTest
But this is a CSS patch :) hitting retest.
Comment #87
timisoreana CreditAttribution: timisoreana at Skilld commentedOn next pages dropdowns works like expected:
1)/admin/structure/menu
2)/admin/config/content/formats
On the next expanded dropdown is cut:
1)/admin/structure/types/manage/article/fields
2)/admin/structure/comment
3)/admin/structure/taxonomy
4)/admin/structure/types
5)/admin/structure/contact
In case of longer (or a few) word(s) in all described pages dropdown is cut
Comment #88
droplet CreditAttribution: droplet commentedThere're 2 options now:
A: #57
B: #63
anyone from UX team can give some feedback ?
Comment #89
andypostComment #90
geek-merlinAdded last patch #82 to summary.
Comment #91
droplet CreditAttribution: droplet commentedLast testing:
Both Patch #63 & #82 doesn't address the problem.
Comment #92
droplet CreditAttribution: droplet commentedAttach a patch for testing
Comment #93
Bojhan CreditAttribution: Bojhan as a volunteer commentedMy preference goes to #57, I dont know why we would want to expand the whole button.
Comment #94
geek-merlinA test of #82 showed me that the css file to patch is
* not (anymore?) core/misc/dropbutton/dropbutton.css
* but core/themes/stable/css/core/dropbutton/dropbutton.css
Comment #95
droplet CreditAttribution: droplet commentedI patched it myself during testing of #82 & #63.
When you reduce the screen width to some points, the menu still cut off.
Comment #96
lokapujyaI think the #57 route is the way to go.
Isn't part of the problem that the columns before the Operations column may push the dropdown off the screen at mobile screen widths? For example, the screenshot in the original issue description. In those cases, the user will either have to scroll to the right, or a new issue can be opened to rearrange that specific table.
If the drop down does fit on the screen, then clicking on it should show all the options (without needing to scroll) and THAT is what I think this issue should be about. So, I think #57 does that. It will extend the dropdown to the left on top of the other columns (while making a selection.)
Most of the time, the first operations drop down text is "Edit". The bug.patch sample is a little unrealistic because an operation name probably shouldn't be too long. So, I think design decisions should be made from real examples from Drupal Admin screens. Not sure how long the options might get in translated text.
Also, what screen width should the testing be done in? I think 320px.
Comment #97
geek-merlinRerolled #82 according to #94 to investigate this. If this can be done in css, it should be.
Comment #98
geek-merlinUpdated summary.
Comment #99
geek-merlinAfter adjusting some non-dropbutton-related style, this patch does what it intended:
Dropbutton always has the width of the largest entry, not wider.
Comment #100
lokapujyaThe #99 patch does what the patch intends to do. So, the next step here is to decide which of the proposed resolutions to pursue or to keep it as is.
Comment #101
droplet CreditAttribution: droplet commented#99 even broken the normal menus
Comment #102
lokapujyaRight, the other problem with solution C is that some drop downs can be very wide, but only have the text "Edit" which would look funny.
Comment #103
joginderpc CreditAttribution: joginderpc at Srijan | A Material+ Company commentedButtons are view fine as per the given text in them, because their would be no much longer text that overlap the block. Please review this.
Comment #105
Maninders CreditAttribution: Maninders at Srijan | A Material+ Company commentedJust use CSS approach to resolve the problem. Please review this patch. Screenshots are also attached with different screen sizes.
Comment #106
Maninders CreditAttribution: Maninders at Srijan | A Material+ Company commentedJust use CSS approach to resolve the problem. Please review this patch. Screenshots are also attached with different screen sizes.
Comment #107
lokapujyaBasically, #105 is proposing to have word break in a dropdown?
Comment #108
Maninders CreditAttribution: Maninders at Srijan | A Material+ Company commentedreferring my #105 patch, just remove the repeat properties and put it generic. Please review this patch. No need of screenshots, effects are same as in #105 attached screenshots.
Comment #109
Manjit.SinghI have reviewed the patch and test the UI locally on Linux.
Please check below screenshot, I have added the extra content to check the UI, But it looks good.
But when there is simple text, I think it should be in single line. Dont know this issues is only in linux or other also. Please check the screenshot.
But when i removing
white-space: pre-line;
it looks good.Comment #110
Maninders CreditAttribution: Maninders at Srijan | A Material+ Company commentedComment #111
Maninders CreditAttribution: Maninders at Srijan | A Material+ Company commentedI have reviewed the patch again its working fine on all machines except Linux. So i changed according to that. Now its working fine on linux also. Please check the screenshots.
Now moving it's to Needs Review.
Comment #112
Maninders CreditAttribution: Maninders at Srijan | A Material+ Company commentedIn last comment i forgot to add patch, sorry for that.
Patch is attached now.
Comment #113
Manjit.SinghComment #114
brahmjeet789 CreditAttribution: brahmjeet789 commentedAfter apply patch #112 the drop down menu go to other side please check the screen shots.
Comment #115
brahmjeet789 CreditAttribution: brahmjeet789 commentedComment #116
kostyashupenkoLooks like this is related with bad styles for tables. I'll look on it soon
Comment #117
kostyashupenkoAdded some fixes. Screen below
After this patch on my side:
Comment #118
brahmjeet789 CreditAttribution: brahmjeet789 commented@kostyashupenko : after apply your patch it works fine for me but there is a issue with header for some screen ,please check my screen shot. we have to work on this.
Comment #119
kostyashupenko@brahmjeet789 about what kind of bug are you talking?
Comment #120
kostyashupenkoA small space between "Manage fields" and breadcrumbs ?
Comment #121
brahmjeet789 CreditAttribution: brahmjeet789 commented@kostyashupenko, I am talking about the header layout get distorted as i marked as red in my screenshot,please check.
Comment #122
Manjit.Singh@kostyashupenko I think he is talking about this.
re-attaching screenshot.
Comment #123
Maninders CreditAttribution: Maninders at Srijan | A Material+ Company commentedMaybe he is talking about the header background color. Which is not 100% covered. @brahmjeet789 Correct me if i am wrong.
Comment #124
Manjit.SinghI have captured some screenshots. Please check.
IMO, It is due to dropdown width which is creating a horizontal scroll on the screen.
@Maninder there is no issue with background color, i guess. If you check the screenshot, you can see that dropdown is slightly out from the body.
Comment #125
kostyashupenkoComment #126
kostyashupenkoWe had this issue, because of too long words. Now property "word-break" will fix that issue. Also now all columns in all tables has the same width (if the screen width less than 600px). Patch and screens here
Comment #127
brahmjeet789 CreditAttribution: brahmjeet789 commented@kostyashupenko i can still find the same problem with header on responsive part for this, please check is it fine on Galaxy S5.
Comment #128
brahmjeet789 CreditAttribution: brahmjeet789 commented@kostyashupenko look at the New Screenshot.
Comment #129
brahmjeet789 CreditAttribution: brahmjeet789 commentedComment #130
kostyashupenkoSorry, my mistake. Should be ok now. I've tested my last patch in IE, Chrome and FF. Everywhere it's ok
So i guess we should discuss smth about "how to improve current design for dropdowns"
Comment #131
andypostLooks good
Comment #132
droplet CreditAttribution: droplet commentedIt isn't a good workaround.
1. It set max-width. On larger screen, it still wrapped the text. (whatever @media we did just based on limited prediction but do not scale for every sites)
2. the fixed max-width just designed for Core styling, let's say if you were taken .layout-container margin away. The menu will be rendered outside the windows. ( If we only designed for Core, opening the menu to left is fine)
3. You should try to test loooooong letter words, not short as #124
Comment #133
kostyashupenkoComment #134
AaronChristian CreditAttribution: AaronChristian at Acro Commerce commentedI'd like to propose getting the D8 Style guide dropbutton redesign implemented then. This would be a great opportunity to do some user validation and testing on the new button.
https://groups.drupal.org/node/283223#Split_Buttons_and_Dropbuttons
This should eliminate the issues noted above, as well as take care of RTL/LTR, mobile devices. I would also suggest keeping in mind accessability and mobile gestures on this button.
Comment #135
droplet CreditAttribution: droplet commentedD8 Style guide for dropbutton is exactly this: http://v4-alpha.getbootstrap.com/components/dropdowns/
which also doesn't cover the edge case we dealing with here.
Patch #130 provided a little bit better workaround than now. We can consider to accept it (as noJS workaround). And a follow-up issue for best solution with JS.
For JS, we can load http://tether.io/ to deal with the positions like Bootstrap does in some components.
(Patch #57 is a lightweight version)
Comment #141
PanchoLet me summarize:
Therefore I went back to the last patch by @axel.rutz in comment #99. His patch ensure the dropbutton is always exactly as wide as the widest element. This however doesn't fix the initial issue (nor my more or less duplicate issue #3029468: Dropbutton field pushed out of the table if no label set) about long dropbuttons being pushed out of the table on crowded pages.
After quite some work, I got it tackled by finally letting the button-wrapper flow inline while accordingly adjusting the child elements.
This new approach fixes the original issue with long multiple dropbuttons being pushed out as well as the mine with single dropbuttons being pushed out. It also slightly reduces the superfluous space taken by short dropbuttons, so the "Operations" column may now move a bit further to the right border (a step towards #1849750: Minimize all "Operations" columns width).
The only thing I'm not yet completely convinced is whether we really want to keep a dropbutton's size fixed at the size of its largest child element, even if closed. For very long child elements this looks a bit weird. If we don't want to change this aspect, we could leave out the @axel.rutz part of the patch.
Further notes:
Now the screenshots:
on filter formats page:
Comment #142
geek-merlinI think a pure css approch is disirable and feasible.
But i'd propose (currently don't have time for that) to iterate the css so that
* a closed dropbutton has the width of the visible item (probably by making the others display:none, not visibility:hidden) (everything else is weird...)
* an open dropbutton has the width of the then visible items (so is wider)
Comment #143
PanchoNo problem, I kind of expected that because of the "veery long dropbutton." I had that before I took into account your patch in #99, and will look if I can still find the version that accomplishes that.
Comment #144
PanchoI first tried a very simple approach that removes the widget's absolute positioning, so even if the widget grows by opening, the table would reflow to give the widget the needed space. I'm sharing the patch with you though I'm not happy with the outcome, as the sudden reflow makes elements move unruly, so this is not something we want to do. We also need to make sure the column doesn't grow, because otherwise that space would be fully consumed by the dropbutton.
Comment #145
PanchoSo if:
1. we want the dropbutton widget to be small at the beginning, but growing with its content
2. we don't want the dropbutton widget take more space than necessary
3. we don't want all other elements to shift unruly, so don't want a full table reflow
4. we don't want the dropbutton widget to grow out of the right page border so it doesn't trigger a horizontal scrollbar
then the only and IMHO certainly our best option is making the open dropbutton widget a fly-over towards the left side.
The details are tricky, too. Now again it's only the heading that does give the column a certain size. In some cases (and languages) the default operation will however have a longer name than the heading, and we don't want the closed widget to already fly over the column to the left. So we need a certain fixed size, and 10 em seemed a good compromise. Now again the heading needs to be right-aligned because otherwise it wouldn't be aligned at all.
The result works fine and looks good in almost every constellation. To get it 100% right I however fear we need JS to calculate the dropbutton width and set the column width accordingly. Unless there is a CSS trick I don't know yet...
See also these two GIFs:
Before:
After:
Comment #146
PanchoActually 7em are enough.
Comment #148
oliverpolden CreditAttribution: oliverpolden commentedThanks for your work on this.
I've been using a dropbutton in a views table. I've applied your patch and found that the dropbutton is the size of the first element within it and overlaps the column(s) to the left when closed. This is happening in both Bartik and Seven themes.
Configuration of the drop button in views would be nice so you can choose to set the closed width and if not set, then to default to the width of the first element.
Comment #149
PanchoThanks for your feedback. We certainly won't manual add configuration options, but have to get this right using CSS and - if necessary - JS. I will do some testing.
Comment #150
geek-merlinAlso see the related issues where similar work is happening...
Comment #151
geek-merlinAnd that one looks like it would solve this and a lot more...
Comment #152
batkorsubscribe
Comment #155
komalk CreditAttribution: komalk at Srijan | A Material+ Company for Drupal India Association commented#146 failed to apply on 9.1.x.
Re-rolled the patch
Comment #156
komalk CreditAttribution: komalk at Srijan | A Material+ Company for Drupal India Association commentedComment #158
Ruchi Joshi CreditAttribution: Ruchi Joshi at Srijan | A Material+ Company for Drupal India Association commentedApplied patch#156 on drupal 9.2, issue is checked with the combination of theme Bartik as default and Seven as admin but drop button text is not retaining its size. Requires to fix. so moving it to "Needs Work"
Screenshot attached.
Steps:
1. Visit /admin/appearance and set Bartik as default and Seven as admin theme.
2. Open this file on local- core/modules/field_ui/src/FieldConfigListBuilder.php
3. Edit it and add any long string under title, this way--> 'title' => $this->t('Storage settings xxxxxxxxxxxxxxxxxxxxxxxxxxxxx'),
4. Visit /admin/structure/types/manage/article/fields on responsive version.
Comment #159
Ruchi Joshi CreditAttribution: Ruchi Joshi at Srijan | A Material+ Company for Drupal India Association commentedComment #160
komalk CreditAttribution: komalk at Srijan | A Material+ Company for Drupal India Association commentedAttached screenshot for reference.
Please review the patch.
Comment #162
djsagar CreditAttribution: djsagar at OpenSense Labs commentedHi komalk,
Patch is applied and resolved the issue just need to update patch.
Thanks!
Comment #163
tanubansal CreditAttribution: tanubansal at Salsa Digital commentedPatch 'PHP 7.3 & MySQL 5.7 27,821' is working fine on 9.1
RTBC+1
Comment #164
djsagar CreditAttribution: djsagar at OpenSense Labs commentedComment #166
mitthukumawat CreditAttribution: mitthukumawat as a volunteer and at Zyxware Technologies for Drupal Association commentedI have applied the patch #160 and it is working fine now. The align of text in dropdown is appearing fine.
Comment #167
VladimirAusHere's what I'm getting after apply the patch on roles pages
/admin/people/roles
Claro
the bahaviour is still jumpySeven
the dropdown is aligned right which doesn't solve the width issue (column header is still aligned right)Tested with
Comment #168
naveen433 CreditAttribution: naveen433 at Valuebound for Valuebound commentedI am working on this
Comment #169
VladimirAusThanks @naveen433.
I created issue here: https://www.drupal.org/project/drupal/issues/3224254
Comment #170
chetansonawane CreditAttribution: chetansonawane commentedComment #171
chetansonawane CreditAttribution: chetansonawane commented