- Issue created by @bronzehedwick
- Merge request !249Issue #3441090. Use inline SVG for drawer Drupal logo. β (Open) created by SKAUGHT
- π¨π¦Canada SKAUGHT
Converts logo.svg from file-system into 'data:image/svg+xml' in navigation_theme. also means we are not using
extension.list.module
service in template hook.other standing work with custom logo will apply still this way!
- Status changed to Needs review
8 months ago 4:05pm 15 April 2024 - Status changed to RTBC
8 months ago 5:50pm 15 April 2024 - πΊπΈUnited States bronzehedwick New York
I think this is a reasonable approach, removing the extra network request. We don't get the added styling benefits, but those don't have a plan to be taken advantage of right now, and this will remove refactoring. Marking RBTC!
- Status changed to Needs work
8 months ago 8:16pm 15 April 2024 - πͺπΈSpain ckrina Barcelona
Thanks for working on this! But I'm not sure about this: moving markup to a .module file makes things strange: as a themer, I'd expect this in a template itself with the rest of the elements if it's not an image. It works? Yes. Is this the expected place for this? Nope :)
I'd go just add the SVG on thenavigation.html.twig
template and add any required logic there if the logo is replaced by a custom image or hidden. - Assigned to bronzehedwick
- Merge request !252Issue #3441090: Use an inline SVG for navigation Drupal logo β (Merged) created by bronzehedwick
- Issue was unassigned.
- Status changed to Needs review
8 months ago 9:24pm 15 April 2024 - πΊπΈUnited States bronzehedwick New York
I created a new MR with a new approach. Is this more inline with what you're expecting @ckrina?
- π¨π¦Canada SKAUGHT
IMO: this does give 'a themer' alot of ability to do things. however, i think we've confusing 'the graphic designer' from 'a themer'.
-the ability to 'change color in twig' is advanced. if we have a color selection in the back end -- this would be usefull. but i might think its 'too much'
-this is completely removing the IMG tag. this will break π Adjust custom navigation logo dimensions on upload Fixed .------
@bronzehedwick. somewhere between. (:*/ function navigation_theme($existing, $type, $theme, $path) { .... $items['navigation'] = [ 'variables' => [ 'hide_logo' => FALSE, 'logo_path' => $logo_path, 'logo_width' => 40, 'logo_height' => 40, 'menu_content' => [], 'menu_footer' => [], ], ];
-. make path empty.
- if twig sees empty load 'new icon twig' into PATH, otherwise PATH would be a custom logo (passed by NaviagaionRenderer). - Status changed to Needs work
8 months ago 12:10pm 16 April 2024 - πΊπΈUnited States bronzehedwick New York
Ah that makes sense, thanks @SKAUGHT. I pushed a commit that adds back that condition.
- Status changed to Needs review
8 months ago 3:24pm 16 April 2024 - Status changed to RTBC
8 months ago 3:27pm 16 April 2024 - Status changed to Needs work
8 months ago 3:45pm 16 April 2024 - π¨π¦Canada SKAUGHT
@bronzehedwick
- moving width/height to var seems not needed [as this doesn't recenter, re-size the actual paths]
- also noticed viewbox is "0 0 32 32" -- i think should be 40? - π¨π¦Canada SKAUGHT
SKAUGHT β changed the visibility of the branch 3441090-use-inline-svg to hidden.
- πΊπΈUnited States bronzehedwick New York
moving width/height to var seems not needed [as this doesn't recenter, re-size the actual paths]
This will re-size/re-center the entire graphic, as intended.
All width/height sizing and any drawing code inside an SVG is relative to the
viewBox
. So the width and height on<rect fill="#347efe" width="32" height="32" rx="8"/>
being set to32
is the same as saying 100% of the width and height, since theviewBox
is 32/32.Increasing the width/height on the
<svg>
element scales everything inside it based on the ratio of theviewBox
, aka the S in SVG ;)also noticed viewbox is "0 0 32 32" -- i think should be 40
As per above, the SVG is displayed at 40/40, but it's scaling baseline is 32/32 (aka, the viewBox). Changing the
viewBox
from 32/32 will make the internal scaling math off, in this case, adding extra white space around the SVG, optically shrinking it.You can test all this by changing the
width
andheight
values on the<svg>
element. - Status changed to Needs review
8 months ago 7:24pm 16 April 2024 - π¨π¦Canada SKAUGHT
width="{{ width|default(40) }}" height="{{ height|default(40) }}"
we just don't need to be allowing the size to be changed. some one could send in a smaller size than the viewbox.re:viewbox. -- i've just got thrown off in my own head (viewport of svg)
- Status changed to Needs work
8 months ago 7:25pm 16 April 2024 - πΊπΈUnited States bronzehedwick New York
Got it @SKAUGHT, I pushed a commit that hard codes
40
as thewidth
andheight
. - Status changed to Needs review
8 months ago 7:41pm 16 April 2024 - Status changed to RTBC
8 months ago 1:50am 17 April 2024 - Status changed to Needs work
8 months ago 3:15pm 17 April 2024 - πͺπΈSpain ckrina Barcelona
This has a conflict with previous changes merged. Needs to be updated
- Status changed to RTBC
8 months ago 3:49pm 17 April 2024 - π¨π¦Canada m4olivei Grimsby, ON
Thanks for the merge conflict resolution @bronzehedwick. Looks good to me.
-
m4olivei β
committed 11b42e3b on 1.x authored by
bronzehedwick β
Issue #3441090: Use inline SVG for drawer Drupal logo
-
m4olivei β
committed 11b42e3b on 1.x authored by
bronzehedwick β
- Status changed to Fixed
8 months ago 3:56pm 17 April 2024 Automatically closed - issue fixed for 2 weeks with no activity.