Recent change to Layout Builder has left Custom Logo Option without it's path and image size.
- Using Drupal 11 dev line, install standard profile.
- enable Navigation module.
- goto 'Navigation settings' config page (admin/config/user-interface/navigation/settings)
- select 'custom logo'
- attach image to 'choose custom logo'
-save config
- notice: Navigation Logo is Still the 'default drupal logo'
Reconnect variable to twig.
#3444391: Center small navigation logo on collapsed state
#3436526: Adjust custom navigation logo dimensions on upload branch will have conflicts now.
N/A.
N/A.
None.
| Comment | File | Size | Author |
|---|---|---|---|
| #35 | 3443810-nr-bot.txt | 2.92 KB | needs-review-queue-bot |
| #16 | cabff9fad625438863c29cb5268f4349.png | 26.26 KB | finnsky |
| #15 | Extend-DrupalPod (1).png | 75.04 KB | dishakatariya |
| #15 | after patch.png | 97.71 KB | dishakatariya |
| #15 | afterpatch.png | 103.49 KB | dishakatariya |
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
ckrinaComment #3
skaughtComment #4
skaught[]
Comment #5
skaughtComment #7
skaughtComment #8
skaughtComment #9
skaughtComment #10
smustgrave commentedShouldn't the tests be included with this fix?
Comment #11
skaughtIn general, yes of course! As this break was so late before the MR and has the standing ticket for related tests, this as a direct repair could be seen to given the work now on dev line.
In general, the tests did shift too with moving to layout builder, I don't have a clear base for where to setup some of the testing. 3443556#35 #3443556: Enhance Navigation admin workflow with Managed Tabs..
Comment #12
divya.sejekan commentedNeed Proper Steps to reproduce and screenshots
Comment #13
skaughtComment #14
skaughtComment #15
dishakatariya commentedHi, I have verified and tested the drupal logo issue in our 11.x dev branch. It is working fine now.
Testing steps:
- Using Drupal 11 dev line, install standard profile.
- enable Navigation module.
- goto 'Navigation settings' config page (admin/config/user-interface/navigation/settings)
- select 'custom logo'
- attach image to 'choose custom logo'
-save config
- notice: Navigation Logo is Still the 'default drupal logo'
Testing Results:
Drupal logo is changing now when to the custom logo being uploaded.
Attached Before and After Screenshots as well.
Can be move to RTBC.
RTBC+1
Comment #16
finnsky commentedwe probably need to define this behaviour:
i uploaded small 5X7 image in tugboat.
Comment #17
finnsky commentedCan be follow up issue i guess.
Comment #18
finnsky commentedFollow up:
https://www.drupal.org/project/drupal/issues/3444391
Comment #19
smustgrave commentedI see the test coverage is in a follow up and this is experimental but are we adding fixes without coverage?
Comment #20
skaughtComment #21
skaughtHave been in communication around #3444673: Navigation Test names are disjointed. and #3443556: Enhance Navigation admin workflow with Managed Tabs.. It is clear that we have the scope to start a new test for this, this way.
Comment #22
skaughtAs I have some other work todo and there is much happening next week, i'll move to needs work so that it may be picked up meanwhile.
I have just added some outlining notes to this WIP in the test page for insights [see testNavigaitonSettingsForm()]. It does not actually test yet (:
Comment #23
ckrinaComment #26
jrperry commentedComment #27
smustgrave commentedThanks for working on this issue! Was previously tagged for tests which are still needed so moving to NW for those.
Comment #28
ckrinaComment #30
skaughtTests for each of the 3 general logo handling options. A bit of a shortcut to setup for op3 to preset the config itself and verify the current fid will be ready for #3436526 and it's tests.
browser_output:
Comment #31
plopescThank you for working on this.
Took a look locally and icon logic works well.
Added some minor comments in the MR.
It would require a review from someone with better knowledge of the HTML structure to confirm that everything is OK.
Comment #32
prashant.cAnother point I noticed that could confuse users configuring the settings is the lack of clarification that these settings are for the 'Site Logo' and not a specific logo for navigation.
It would be helpful to add a brief description to each option or a general help text to clarify the purpose of these settings.
Thank you.
Comment #33
skaught#32, indeed there are some open conversations around this.
#3448729: [meta] Improve the navigation layout page maybe the best issue to point you to.
Comment #34
skaughtComment #35
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #36
skaughttest are running green again.
Comment #37
plopescRemoved a couple of unnecessary dependencies from test class.
I think it is safe to mark this as RTBC now.
Thank you for your efforts here!
Comment #38
nod_Question about the variables available inside the template and comment line length to fix.
Comment #39
skaughti'll return to RTBC. thanks!
Comment #40
larowlanLeft some questions on the MR - thanks for working on this!
Comment #41
skaughtI'll returning to RTBC again, thanks!
Comment #42
skaughtComment #43
nod_Comment #49
nod_Committed and pushed f479374ee8 to 11.x and 85a4d771c1 to 11.0.x and 58c748be8e to 10.4.x. Thanks!