Open Toutouwai opened 6 years ago
Another column width issue (not sure if it's the same root problem or if this should be a separate issue)...
Even when there are no show-if dependencies and all fields are visible in a row, the widths that are applied in CSS are not the same as the widths specified in the field settings. As shown in the screenshot above, the column widths are 30%, 35%, 35% - a total of 100% so should fill the row completely (and does in AdminThemeDefault and AdminThemeReno).
But the effect in AdminThemeUikit is:
This is known issue. AdminThemeUikit doesn't work like other themes. Change the widths to 33/33/34. See https://github.com/ryancramerdesign/AdminThemeUikit/issues/8
AdminThemeUikit doesn't work like other themes. Change the widths to 33/33/34.
Well that's a real step backwards and will break a lot of existing Page Edit layouts (and possibly module config layouts) if/when the admin theme is changed to AdminThemeUikit on existing sites. I don't have much appetite to go through hundreds of templates on dozens of sites to check if the all the column widths conform to these new restrictions.
Why can't the column widths be set via JS to the widths specified in the field settings? There wasn't any problem with the old system for setting column widths, was there?
Thanks, I have fixed the issues outlined here and they will appear on the updated version of AdminThemeUikit which I'll add to the core later this week.
Regarding the 30/35/35 percentages, it's true that this theme works differently than the others, because it converts the widths to existing predefined uk-grid classes from the Uikit framework. However, when a new combination is found that doesn't work, I can usually make updates to map them properly to prevent any layout problems like in your screenshot. For the case you mentioned, I updated it so that rather than 30/35/35 mapping to 1-4, 1-3, 1-3, it now maps to 1-3, 1-3, 1-3.
Regarding the "uk-grid classes", I found that UIkit 3 has only 6 columns, therefore it is impossible to set more than six fields in a row. Unfortunately this is a major drawback, I do not know what to propose to fix it. Because of this, I switched back to Reno for the time being.
Another BIG issue with AdminThemeUikit 3 is the lot of whitespace. It would be preferable to set margins+padding to that of Reno, I mean visually the same whitespace, of course. Data intensive layouts are worse in AdminThemeUikit because a lot less fits in the same screen real-estate (ie viewport).
Regarding the 30/35/35 percentages, it's true that this theme works differently than the others, because it converts the widths to existing predefined uk-grid classes from the Uikit framework.
I know that front-end frameworks like Uikit come with grid classes but there's no obligation to use them in all circumstances, is there? My understanding is that they exist as timesaving tools, but where you want greater flexibility or precision you either override them or just don't use them.
In the case of translating a column width value defined as a percentage in the PW admin/API to a limited number of Uikit grid widths we end up with a less useful system for form layouts than we had previously. I can't see why we would want to do that in our new whizz-bang admin theme.
Another BIG issue with AdminThemeUikit 3 is the lot of whitespace. It would be preferable to set margins+padding to that of Reno, I mean visually the same whitespace, of course.
It's relatively easy to fix this in your own custom CSS overrides. I have done this myself and there are not that many rules you need to override. I've come around to the idea that it's not likely every admin theme styling decision will suit every user. So needing to add custom CSS tweaks that suit your own tastes is to be expected.
Dealing with the column width situation with custom admin JS will not be so easy though. I haven't tackled it yet but I can see there being problems with the custom JS fighting the core JS. Maybe @ryancramerdesign would consider breaking off the column sizing JS into a separate file so it can be replaced by those who prefer something different (if it comes to that).
I know that front-end frameworks like Uikit come with grid classes but there's no obligation to use them in all circumstances, is there? My understanding is that they exist as timesaving tools, but where you want greater flexibility or precision you either override them or just don't use them.
In the case of translating a column width value defined as a percentage in the PW admin/API to a limited number of Uikit grid widths we end up with a less useful system for form layouts than we had previously. I can't see why we would want to do that in our new whizz-bang admin theme.
I agree. I actually don't see any real benefit in Uikit admin theme. The most usefull thing is that we can add custom logo and set up avatar. What we need is some guidance on how we can modifiy the theme eg. where and how we can change that spacing (and other things) without having to look/analyze wire/modules/AdminThemeUikit/uikit/dist/css/uikit.pw.css.
This discussion is getting a bit OT, sorry for that, but just one more thing about AdminThemeUikit: the purpose of such a library would be to enable easy customization which can be envisioned in two ways (both can be utilized at the same time):
The latter (2.) can be achiever by documenting how to do it but such a customization method should support overriding things in a way that possible breaking changes of future system updates can be dealt with on the customization side too (ie. the developer is provided with enough information about where to look in order to fix override issues introduced by breaking changes).
Using Uikit 3 is only preferable if we know how to customize it, so we definitely need documentation to show us the "official way" of doing it.
@ryancramerdesign May I ask if you are planning to deal with this related issue or not? : "UIkit 3 has only 6 columns, therefore it is impossible to set more than six fields in a row." I do not think AdminThemeUikit should be officially recommended before fixing this as switching to it breaks page layouts depending on such a setup which is historically possible and supported with both Reno and Classic themes.
Hopefully I won't sound too arrogant here, but I have literally never used more than 4 fields on a row, and I have hard time seeing how more than 6 fields would be usable. Is this a use case that comes up often?
I'm not trying to downplay the problem of backwards compatibility, but in some cases introducing non-bc stuff is still the most reasonable thing to do: Uikit is, in many ways, an improvement, so we'd have to weight in if it really makes sense to drop it's grid because of this.
Unless, of course, there's an easy fix for this that doesn't require dropping the grid and still allows us to remain compatible with future Uikit updates etc.? :)
"Is this a use case that comes up often?" I don't know, but I have a bunch of integers to fill in, of which 10 fits nicely in a row. I have never had the need to use multisite – for instance – but I hope support for it will never be dropped, just because ;)
I've also never used more than 4 fields in a row, but my screen is kind of small, and more than that isn't practical. Whether we need it or not, I don't really know why Uikit v3 stopped at 6 columns. Uikit v2 supported 10 columns. If they don't bump up the column count before Uikit 3 final, and there is demand for it, I think we can add the appropriate CSS classes to do our ourselves. I seem to remember in Uikit 2 you could add a 3rd party file to make it use a 12 column grid instead of 10, and perhaps something similar will be available for Uikit 3, but I haven't found anything like that yet. Letting Uikit handle the grid makes the admin theme JS quite a bit simpler and more maintainable. But if someone needs something like 10 fields in a row in PW admin then sticking with the default or Reno admin would be recommended for that case, at least in the short term. But the plan is that we will add whatever additional CSS classes are necessary to support a larger number of columns in the Uikit admin theme. I just wanted to wait till Uikit 3 was final before hacking away at it.
Last time I googled I could not find it but this time I have: UIKit Extra Widths: https://github.com/OwenMelbz/uikit3-extra-widths and the related discussion: https://github.com/uikit/uikit/issues/2593 Maybe it is still usable even if it is "old". I will also test when I have the time.
@ryancramerdesign, the issue as described in the first post remains for me in v3.0.93.
The issue as best I can describe it... In AdminThemeDefault and AdminThemeReno a row of inputfields consists of two or more adjacent inputfields whose widths sum to 100% (or close to it - personally I always like to get an exact 100% sum). When one or more of those inputfields in a row is hidden (e.g. because of a visibility dependency) then the last visible inputfield in the row expands to occupy the remaining space. This is a very useful feature, and I think a lot of people are using it for their Page Edit layouts.
AdminThemeUikit doesn't do this. Inputfields just stay at their designated width (or rather the closest corresponding uikit width - more on that in a minute) when other inputfields in their row are hidden. That means that rows are not maintained, resulting in gaps or inputfields below jumping up into the previous row. The screen captures below illustrate the difference.
AdminThemeDefault/AdminThemeReno:
AdminThemeUikit:
Regarding the rounding of designated inputfield widths to a limited number of preset uikit grid widths: I can only really repeat what I said in a comment above. It's less flexible than the previous system for column widths and I just can't see why it's necessary to change to it.
As I understand it, the preset grid width features that come with Uikit are entirely optional. If they don't suit what is desired (and in the case of Page Edit layouts I think that is the case) then this can be handled in one of two ways.
(1) Use the Uikit width classes on the elements, but override the widths to what is desired. This Codepen illustrates what I mean: https://codepen.io/toutouwai/pen/VQqemo
Here I am using the width class uk-width-1-2
but overriding it to what I want, which is column widths of 40% and 60%. I think this is a pretty normal thing to do where you want finer grained control of an element. Can't we do the same in Page Edit? Use the column width system from AdminThemeDefault/AdminThemeReno where widths are set by JS according to the designated width values for the inputfield, plus the "fill the row" magic mentioned previously.
(2) Simply don't use the Uikit width classes. As far as I know there is no law in Uikit that says that every element must use a width class. You can use elements with custom classes and custom styles anywhere you want together with any Uikit features. Uikit is meant to be a helping hand for front-end development but it doesn't replace all the things you would do in "normal" front-end development without Uikit. You use the Uikit features where they're helpful and don't use them where they're not helpful.
But truth be told I don't use front-end frameworks such as Uikit so maybe someone who uses them can explain it to me why it has to be this way.
"This is a very useful feature, and I think a lot of people are using it for their Page Edit layouts." Sure, I use it too. https://getuikit.com/docs/width#mixing-widths Is it what we need?
<div class="uk-child-width-expand uk-grid-small uk-text-center" uk-grid>
<div class="uk-width-1-6">
<div class="uk-card uk-card-default uk-card-body">1-6</div>
</div>
<div class="uk-width-1-6">
<div class="uk-card uk-card-default uk-card-body">1-6</div>
</div>
<div class="uk-width-1-6">
<div class="uk-card uk-card-default uk-card-body">1-6</div>
</div>
<div>
<div class="uk-card uk-card-default uk-card-body">Expand</div>
</div>
</div>
Classes must be added and removed on the fly, of course, but it should work I guess.
the issue as described in the first post remains for me in v3.0.93.
@Toutouwai It's working for me here, maybe I've missed something? Can you try editing the /wire/modules/AdminTheme/AdminThemeUikit/config.php file? At the bottom of it is some commented out code that I'd put in to duplicate and fix the this issue report. This code is still there on the current dev branch, it just needs to be uncommented:
// The following is just for development/testing
$f = $modules->get('InputfieldRadios');
$f->attr('name', 'test_radios');
$f->label = 'Test radios';
$f->addOption(1, 'Option 1');
$f->addOption(2, 'Option 2');
$f->addOption(3, 'Option 3');
$f->columnWidth = 30;
$inputfields->add($f);
$f = $modules->get('InputfieldText');
$f->attr('name', 'test_text1');
$f->label = 'Test text 1';
$f->showIf = 'test_radios=1';
$f->columnWidth = 35;
$inputfields->add($f);
$f = $modules->get('InputfieldText');
$f->attr('name', 'test_text2');
$f->label = 'Test text 2';
$f->showIf = 'test_radios=1|2';
$f->columnWidth = 35;
$inputfields->add($f);
After you've uncommented the above, go to Modules > Core > AdminTheme > AdminThemeUikit. You should see the test scenario at the bottom of the module configuration screen. It's working perfectly in testing here, but maybe I've missed some aspect of your test case?
@ryancramerdesign, a couple of things...
(1) There seems to be some difference between how the inputfield widths are applied in Page Edit versus the module config example. When I uncomment the code you mentioned the widths do work as expected in the module config screen. But effectively the same setup in a template does not work as expected in Page Edit. Not sure what could be the difference though. Here are the template fields: The inputfield dependency for text_1 (text_2 is the same but has options=2): The result in Page Edit:
(2) Neither the module config example or Page Edit works as expected when there are fields in a row below. For example, if you modify the code in the module config to...
// First row of fields
$f = $modules->get('InputfieldRadios');
$f->attr('name', 'test_radios');
$f->label = 'Test radios';
$f->addOption(1, 'Option 1');
$f->addOption(2, 'Option 2');
$f->addOption(3, 'Option 3');
$f->columnWidth = 30;
$inputfields->add($f);
$f = $modules->get('InputfieldText');
$f->attr('name', 'test_text1');
$f->label = 'Test text 1';
$f->showIf = 'test_radios=1';
$f->columnWidth = 35;
$inputfields->add($f);
$f = $modules->get('InputfieldText');
$f->attr('name', 'test_text2');
$f->label = 'Test text 2';
$f->showIf = 'test_radios=2';
$f->columnWidth = 35;
$inputfields->add($f);
// These inputfields should appear as a second row
$f = $modules->get('InputfieldText');
$f->attr('name', 'test_text3');
$f->label = 'Test text 3';
$f->columnWidth = 30;
$inputfields->add($f);
$f = $modules->get('InputfieldText');
$f->attr('name', 'test_text4');
$f->label = 'Test text 4';
$f->columnWidth = 35;
$inputfields->add($f);
$f = $modules->get('InputfieldText');
$f->attr('name', 'test_text5');
$f->label = 'Test text 5';
$f->columnWidth = 35;
$inputfields->add($f);
...then the result is...
@Toutouwai Thanks for that test case, I can duplicate the issue using the code you pasted above, so will work in this.
@ryancramerdesign - I am still seeing these issues when showif restrictions prevent a field from being shown - the one from the next row gets promoted to the current row. This completely messes with the logic of separating groups of fields. I know we have fieldsets, but that seems overkill (and doesn't work in the User template: https://github.com/processwire/processwire-issues/issues/509) and I'd need nested fieldsets in my case. What about having a "separator" field that functions like clear:both
so that you always end up on the next row even if the previous row is not complete?
@Toutouwai I've just pushed an update that should correct the issue you mentioned above. Your code for that test remains in the AdminThemeUikit config.php file if you want to uncomment and test it. Also, in the page editor, I setup an options field with two text fields exactly as you did, but everything works there as expected. Let me know if you are still seeing it not work in the page editor vs. elsewhere.
@adrianbj that was related to the issue Toutouwai mentioned, so it should be fixed now.
@ryancramerdesign, thanks for the update. It is working better now, but still not in the way expected. In the case where there are three fields in one row...
...field 2 appears as expected when made visible by field 1 - it expands to fill the remaining row space. But when field 3 is made visible by field 1 it does not expand, and instead field 1 expands. The expectation is that field 2 and field 3 behave the same way when made visible.
The screen capture below is from Page Edit, but I tested the setup in a module config and the result is the same.
Another issue I spotted: two fields in a row with widths 55% and 45%. This results in an incompletely filled row.
@Toutouwai if I understand correctly, you want it to apply the hidden width to the next visible Inputfield rather than the previous, unless the hidden Inputfield is the last in the row, in which case it applies it to the previous. I've just pushed an update that does this. Also corrected the 55/45% combo.
Sorry to report that today's updates actually seem to make things worse here. There are 5 fields set at 20% and now the 5th one ends up on the next row.
@ryancramerdesign, I'm hoping for the same behaviour as the Default and Reno themes. I think the logic there is something like:
"Where the sum of the widths of the visible inputfields in a row is less than 100%, find the last visible inputfield in the row and increase its width to make up a total of 100%".
"I'm hoping for the same behaviour as the Default and Reno themes" Needless to say that I also consider being able to use 10 "columns" ie. inputfields in a row to be "the same behaviour" as before. I wish I could help but I'm so bogged down with work these days that I only have time to repeat myself :(
Anyway, I fear that if the "10 inputfields in a row" issue is not solved this time it might be very problematic to implement the support for it in the future.
Sorry to report that today's updates actually seem to make things worse here. There are 5 fields set at 20% and now the 5th one ends up on the next row.
@adrian so far I can't duplicate that. I'm getting 5 on a row at 20%. What's different in your case is that you've got a lot of different Inputfield types in there, though I don't think that should matter. What are the width settings for the field immediately before the row that you showed? Also, what are the width settings for that last field that shows the "View" link in it? Can you try with an Incognito window or with browser cache disabled, just to make sure it's not an older file version sticking around?
Anyway, I fear that if the "10 inputfields in a row" issue is not solved this time it might be very problematic to implement the support for it in the future.
I'm not sure I understand this statement. See my earlier reply where I mentioned that if they don't bump up the column count to 10 before Uikit 3 gets out of beta, I see no problem with adding our own classes (like that uikit3-extra-widths you linked, if it still works). Since you are interested in this capability, it might be helpful if you started experimenting with it now to let us know how it goes. Try adding this to the top of your /site/templates/admin.php file:
$wire->addHookBefore('ProcessController::execute', function($e) {
$config = $e->wire('config');
$ukGridWidths = $config->js('ukGridWidths');
if(empty($ukGridWidths)) return; // in case not Uikit admin
$ukGridWidths['10%'] = '1-10'; // add your class(es) like this
$config->js('ukGridWidths', $ukGridWidths); // add to JS
$config->InputfieldForm('ukGridWidths', $ukGridWidths); // add to forms
$config->styles->add('/url/to/uikit3-extra-widths.css'); // add custom CSS file
});
Hi @ryancramerdesign - I think the issue might be because my fields are inside a fieldset. Can you please try that to see if you can reproduce. If not, I'll post more details. There's just lots of sensitive details (including field names) that I have to mask.
@adrianbj I tried a fieldset, but that doesn't appear to be it either (see my screenshot below). No need to post field names or any other details. I only need to know the widths of the fields. If you can post a screenshot similar to your earlier one that just showed the fieldtype and width percentages from ProcessTemplate, that might help (or just type the widths if you prefer). I only need the widths, but for all the fields rather than just the row of five (or at least the fields in a couple rows above and below the one you posted earlier). For instance, in my screenshot, the fieldset has no width (100%), the first row within it is 35% 65%, the second row is all 20%, and the 3rd row is 75% 25%.
Here you go:
Let me know if you still need more details. Thanks!
@adrian Thanks. I didn't notice before that one of those 20% width columns has a showIf condition on it. Is it the Page field above it that causes it to conditionally appear or not?
Yes, it is one of the page fields above - it's the first one just inside the fieldset.
Try adding this to the top of your /site/templates/admin.php file: $wire->addHookBefore('ProcessController::execute', function($e) {...
I did it and even though the css is loaded I do not know where to go from there. I mean, I have inputfield widths set to 10% but in the page source I get uk-width-1-6@m
which means not much has changed and do not know what should have changed. I'm really stuck with it, I do not get what I should do.
BTW, the description of the Column Width slider reads: "The percentage width of this field's container (10%-100%)...." If AdminThemeUikit – by any change – will not support 1/7 columns and below, then this hint is misleading as 16.6% is the smallest supported. So it should read (16.6%-100%) I guess.
@szabeszg edit /wire/modules/AdminTheme/AdminThemeUikit/AdminThemeUikit.module and replace 16 with 10 in line 214:
if($columnWidth < 16) $columnWidth = 16
Hey @ryancramerdesign - just wondering if you need anymore information from me to figure out the mentioned issue above? Thanks.
I did it and even though the css is loaded I do not know where to go from there. I mean, I have inputfield widths set to 10% but in the page source I get uk-width-1-6@m which means not much has changed and do not know what should have changed. I'm really stuck with it, I do not get what I should do.
See @matjazpotocnik note above, I think that's likely what you need to adjust. I am updating AdminThemeUikit.module to detect the min column width from the $config->ukGridWidths rather than having it hard coded in there.
BTW, the description of the Column Width slider reads: "The percentage width of this field's container (10%-100%)...." If AdminThemeUikit – by any change – will not support 1/7 columns and below, then this hint is misleading as 16.6% is the smallest supported. So it should read (16.6%-100%) I guess.
Reno and Default support this value. If we don't end up supporting down to 10% in Uikit, then maybe we can update that description, but at this point it seems like there's a good chance we'll be able to support it if you find it works well in testing.
Hey @ryancramerdesign - just wondering if you need anymore information from me to figure out the mentioned issue above? Thanks.
I was able to duplicate and fix the issue you mentioned above with 5 20% columns and the first one having a showIf condition. It was that showIf condition that I missed before. I'm just testing to make sure the fix didn't affect anything else before I commit it.
I was able to duplicate and fix the issue you mentioned above with 5 20% columns and the first one having a showIf condition. It was that showIf condition that I missed before. I'm just testing to make sure the fix didn't affect anything else before I commit it.
Hey @ryancramerdesign , glad it looks like you might have this one sorted. I have to demo that site to the client tomorrow, so if you feel like sharing the new version over PM, it would be very helpful - thanks!
@adrianbj the latest commit referenced above should have that fix
Hey @ryancramerdesign - now it's a new issue. Notice the width of the postcode/zip field. In this example, the email field is hidden by a showif, but the postcode is not expanding to fill up the row, so the subscriber field is showing a row above what it's meant to be on which makes things even worse really :(
I just tested this as well and I'm seeing a problem with fields that are 50% width. They are to big. See the image below:
I tested in an incognito window just to make sure the browser wasn't caching anything.
@ryancramerdesign - any thoughts on my idea of making it possible to add a row separator? I feel like this would make things much easier in the end even if it's a bit more work now, especially trying to integrate it into ASM as something that can be added multiple times.
It's this commit: https://github.com/processwire/processwire/commit/20f02cddf9b200a91f798630753a15b8b0c22290
$minColumnWidth = (int) reset($widthKeys);
should be
$minColumnWidth = (int) end($widthKeys);
Oh, Ryan already fixed it ...
Just tested the latest. That fixed the 50%/50% issue. Thank you
@ryancramerdesign - any thoughts on my idea of making it possible to add a row separator? I feel like this would make things much easier in the end even if it's a bit more work now, especially trying to integrate it into ASM as something that can be added multiple times.
+1
I've had problems with column widths and showIf contitions multiple times, where fields that should belong in the next row jumped in the upper row when a field there was hidden via showif...
@ryancramerdesign changing ksort() to sort() worked, but there is still an issue with ajax loaded fields:
rd1 and rd2 are ajax loaded 50% and too small. It works when I set them to "open" by default. A spacer (linebreak) field like shown in the screenshot would be great as well (a hidden one, of course)
@BernhardBaumrock Thanks I have fixed that in the above commit. It wasn't specific to AdminThemeUikit actually, it was render() hooks that were getting skipped over in InputfieldWrapper and just wasn't apparent before.
Regarding row separator, can consider that feature request on future dev branch, but just focused on getting everything perfect for this master version at the moment.
@adrianbj I can't seem to duplicate the issue you mentioned, though not exactly sure I've setup my test case correctly, as I don't know how to map the fields in your screenshot to the list of types/widths screenshot you posted earlier. However, I'm wondering if you might have seen a side effect from where I had an incorrect ksort() for a few hours yesterday, which should have been a sort(), which is fixed.
thanks, works 👍
I understand and like the feature freeze strategy. But it would be great to get the separator in the future! Maybe this could also be an option for AOS @rolandtoth ? He already built a vertical separator: https://processwire.com/talk/topic/17278-wordpress-vs-processwire-side-by-side-briefly/?do=findComment&comment=151735
@ryancramerdesign - I think you might have forgot to add this commit https://github.com/processwire/processwire/commit/d82d011ecac1361bf30021faf4c19a2f657f9751 to the adminThemeUikit repo https://github.com/ryancramerdesign/AdminThemeUikit as well.
Short description of the issue
Template fields and column widths are as below:
Because of the column widths, fields
options
,text_1
andtext_2
should all appear on the same row. Fieldstext_1
andtext_2
have visibility dependencies set: each is shown if theoptions
field has a certain value, and are never shown both at the same time. In other words,options
determines iftext_1
is shown or iftext_2
is shown.AdminThemeDefault works as expected: AdminThemeReno works the same.
But AdminThemeUikit does not set the column width of the inputfields correctly:
Expected behavior
All admin themes calculate and display column widths the same.
Setup/Environment