-
Notifications
You must be signed in to change notification settings - Fork 14.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[fix] Support APP_ICON_WIDTH configuration parameter in SPA menu #9666
Conversation
… bootstrap for use in Menu
Codecov Report
@@ Coverage Diff @@
## master #9666 +/- ##
==========================================
+ Coverage 64.03% 70.52% +6.49%
==========================================
Files 532 581 +49
Lines 28897 30216 +1319
Branches 2758 3072 +314
==========================================
+ Hits 18503 21311 +2808
+ Misses 10211 8793 -1418
+ Partials 183 112 -71
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you set the width in the config file to 126? Then we'll both have documentation of the config var existing, and can replace the config.get
with a config["APP_ICON_WIDTH"]
because we can always assume it exists
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
It appears There are a handful of other config.get instances that pull in existing vars from |
We've tried to standardize around |
…of .get and adjust Menu proptypes to be required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, thanks for the fix!
CATEGORY
Choose one
SUMMARY
The SPA menu component was relying on a hardcoded icon width of 126. This change exposes the config.py APP_ICON_WIDTH property to the frontend through the apps bootstrapping process and alters the SPA menu component to consume this value as the logo image width.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
n/a
TEST PLAN
Set APP_ICON_WIDTH property to a value other than 126 and observe the SPA and template version of the header have matching logo sizes.
ADDITIONAL INFORMATION
REVIEWERS