- Issue created by @catch
- 🇬🇧United Kingdom catch
Marked ✨ Allow SVG logos Closed: duplicate duplicate since this issue would enable svg logos to be used.
- First commit to issue fork.
- Assigned to pooja_sharma
- Merge request !9337Resolve #3462829 "Store the filepath for navigation logo" → (Open) created by pooja_sharma
- Issue was unassigned.
- Status changed to Needs review
11 days ago 2:23pm 27 August 2024 Implemented code to support image path instead of file id of navigation logo, also made some adjustments to the respective code files for the new changes. apart form nothing seems to be left.
Please review , moving NR.
- Status changed to Needs work
9 days ago 11:03pm 29 August 2024 - Assigned to pooja_sharma
- Issue was unassigned.
- Status changed to Needs review
8 days ago 10:51am 31 August 2024 Addressed the feedback, left comment on MR. PLease review, moving NR.
- Status changed to Needs work
7 days ago 1:40pm 31 August 2024 - Status changed to Needs review
7 days ago 3:43pm 31 August 2024 Addressed the feedback, left comment on MR. PLease review, moving NR.
- 🇺🇸United States smustgrave
I don't want to keep playing ping pong with this. Will leave for somewhere else but -1 from me as we are hard coding values and schema validation appears to be missing.
- Assigned to pooja_sharma
- Status changed to Needs work
7 days ago 5:13pm 31 August 2024 On adding validation constraints appears random test failures , moving to NW 'll debug it.
hard coding values of logo extension now added in config & added constraints validation as well
As new key
logo_extension
added, it leads to 'validation config' warning: https://git.drupalcode.org/issue/drupal-3462829/-/jobs/2613092But not getting similar warning on local even try with this command:
drush config:inspect --filter-keys=navigation.settings --detail --list-constraints
if 'll be great if anyone can share suggestion for it.
- "navigation.settings": 4, + "navigation.settings": 5, "node.settings": 2, "node.type.*": 22, "olivero.settings": 16, @@ -2788,10 +2788,10 @@ }, "perPropertyPath": { "all": { - "count": 17140 + "count": 17141 }, "validatable": { - "count": 10377 + "count": 10379 }, "fullyValidatable": { "count": -1
- Issue was unassigned.
- Status changed to Needs review
6 days ago 5:08am 2 September 2024 Tried to address the mentioned feedback, apart form it nothing appears to be left.
MR is mergeable, there is one warning validation config, it display validation count & line no. increase which is correct, no any error message displaying, not sure on the basis of it, PLease review, moving to NR.
- 🇮🇳India KumudB Ahmedabad
There is an issue occurring when clicking on the entity used for the logo from the file system.
1. Upload a custom logo:
Navigate to Administration > Configuration > User Interface > Navigation > Settings.
Upload and save your custom logo.2. Save the configuration:
Ensure the changes are saved properly.3. Verify the issue:
Go to Administration > Content > Files.
Locate your most recently uploaded logo file.
Click on the Usage link for this file.
Observe the error that appears.The website encountered an unexpected error. Try again later.
Drupal\Component\Plugin\Exception\PluginNotFoundException: The "logo" entity type does not exist. in Drupal\Core\Entity\EntityTypeManager->getDefinition() (line 138 of core/lib/Drupal/Core/Entity/EntityTypeManager.php).
@kumudb, This issue not replicate due to these MR changes , I have verified & created issue here 🐛 Page break error display :The "logo" entity type does not exist Active , can track there.
- 🇺🇸United States smustgrave
Will leave for additional reviews
But my thoughts
But logo_managed variable name for a path doesn't make much sense to me
Logo_extensions seems to have no field (is there not a way to get allowed file uploads?)
The schema for logo_managed says it's allowed to be null but if it's a path to an image then should validate it's a valid path. But logo_managed variable name for a path doesn't make much sense to me
Agree , 'll working on the same, keep logo_path variable name.
Logo_extensions seems to have no field (is there not a way to get allowed file uploads?)
Generally when we upload the image on the node form , there is only visible image field, & image extension not visible to the user , it is attached with field storage.
Here if we 'll add logo_extension field then on the same form logo extension & logo upload field both 'll be visible, this 'll be bit confusing experience for the user.
The schema for logo_managed says it's allowed to be null but if it's a path to an image then should validate it's a valid path.
On the basis of my understanding trying to rply correct me if I 'm missing anything, 'logo_managed' can be null because at the BE there are three opts available: default: (by default this value is selected when this selected then image path not get from the config, so image path not store as well, due to which I set null ) ,
hide: in this opt no image 'll display & not store it's value in config, custom logo update: only in this opt image store, so on the basis above mentioned scenarios keep it null.