LB (Ben Johnston)
LB (Ben Johnston)
I think this is looking good. I wonder if we should maybe reverse the default and then add upgrade considerations. Depending on timing for 6.0 though.
@nandini584 thank you. Could you please look at the failing test ``` ====================================================================== FAIL: test_adapt (wagtail.admin.tests.ui.test_sidebar.TestAdaptMainMenuModule) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/runner/work/wagtail/wagtail/wagtail/admin/tests/ui/test_sidebar.py", line 235, in test_adapt self.assertEqual( AssertionError:...
Ohh wait, I think we DO want to change the default (re-reading the previous PR), so the failed test may need to be updated to reflect the new behaviour.
@nandini584 just checking, it would be good for you to also have a full read of the previous PR's comments. I did a review of that suggesting a different approach...
Thanks @Temidayo32 I may not get to this for another week or so - but pinging @thibaudcolas if you have some time to look at this.
Rough idea, not sure if this will work but might be a good starting point. 1. Rework where the default/size are declared to closer to where we need them. 2....
`wagtail/admin/tests/ui/test_sidebar.py` will also need to be updated when this is done as it tests the full avatar URL for some reason. ``` "avatarUrl": "//www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=100&d=mm", ```
Gravatar will ignore the `d` Param if you also pass in `default`, so the configured url could override the `d`. However, I think adding the kwarg also is fine but...
@nandini584 it looks like you have included the entirety of the `mysite` folder in your PR. I'd recommend you remove it, then squash the commits so it's not on the...
@terrsoshi thank you for this PR. Are you able to add unit tests for the new behaviour so that it does not break in the future, are you also able...