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
Create Default Nav Links In New Forem (RFC #237) #14345
Conversation
lib/tasks/add_navigation_links.rake
Outdated
@@ -1,25 +1,69 @@ | |||
# rubocop:disable Metrics/BlockLength | |||
namespace :navigation_links do | |||
reading_icon = File.read(Rails.root.join("app/assets/images/twemoji/drawer.svg")).freeze |
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.
It's probably just code golf, but you could make a local helper method for image_content(*paths)
and save a few bytes - the image_content
exists for the one rainbowdev image not in the twemoji directory.
def image_content(*paths)
File.read(Rails.root.join("app/assets/images/#{paths.join('/')}")).freeze
end
def twemoji(name)
image_content("twemoji", "#{name}.svg")
end
reading_icon = twemoji("drawer")
contact_icon = twemoji("contact")
...
I'm sure this achieves the "unlikely to have a typo on only one line" goal of removing duplication while providing little concrete value in added readability.
…om:forem/forem into msarit/create-default-nav-links-new-forems
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.
Looks good to me! 🚀 Also, TIL what a "twemoji" is. 😄
What type of PR is this? (check all applicable)
Description
As mentioned in the Static Nav Sections PR, this PR ensures that new forems have the default links prescribed by the Static Sections RFC.
❓❓ WHY do I have so many commits? I tried rebasing but that just added MORE commits! 😫
QA Instructions, Screenshots, Recordings
Run specs locally and ensure they pass.
UI accessibility concerns?
None.
Added/updated tests?
have not been included
[Forem core team only] How will this change be communicated?
Admin Guide, or
Storybook (for Crayons components)
or in a forem.dev post
replace this line with details on why this change doesn't need to be
shared