- Merge request !59Allow a minimum number of breadcrumb segments to be present in order to render breadcrumbs. β (Merged) created by ericgsmith
- last update
9 months ago 6 pass - π³πΏNew Zealand ericgsmith
Apologies for taking over a year to respond - I appreciate the review and the feedback.
Coming back to this after a year - the intent of setting the config to 1 is to maintain existing functionality. I believe its good practice to make sure newly introduced config includes an update to set the default value, otherwise we can make the schema nullable. Setting the value to 1 is not changing any existing behaviour.
I do find the UI confusing - and I think when you say "can check the box" when they install, that it looks like the other settings on the form are related / have control over this. That is my mistake for positioning the field next to that - I didn't really look close enough at the existing behaviour of that option first. That "Limit breadcrumb trail segments" is from #3174165: How to support limiting depth β and to be honest I'm still a bit confused by it - because I think the wording of the label and the functionality is a bit different π But, I don't want to confuse this feature request with that functionality - I think it will be clearer if I make the UI clearer what this field does.
Perhaps it would be better to move this new field down into the Advanced settings - I prefer the UX of the form fields there that are enabled by a checkbox and then have the field value only shown when checked. I could refactor it so its like this:
I think that wording makes it a bit clearer too?
- πΊπΈUnited States Greg Boggs Portland Oregon
ok got it! Sounds like we can include this as is.
47:27 32:55 Running-
Greg Boggs β
committed 15319fe5 on 2.x authored by
ericgsmith β
Issue #3309109 by ericgsmith: Add option to set a minimum segment length...
-
Greg Boggs β
committed 15319fe5 on 2.x authored by
ericgsmith β
- Status changed to Fixed
9 months ago 2:51am 5 April 2024 - π³πΏNew Zealand ericgsmith
Ah thanks @Greg - I just pushed with the suggested change - all good if we don't need it - thanks for the review!
-
Greg Boggs β
committed 15319fe5 on feature/megre_dynamic-breadcrumbs authored by
ericgsmith β
Issue #3309109 by ericgsmith: Add option to set a minimum segment length...
-
Greg Boggs β
committed 15319fe5 on feature/megre_dynamic-breadcrumbs authored by
ericgsmith β
Automatically closed - issue fixed for 2 weeks with no activity.