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
Recommend using target="_blank" instead of target="blank" (incorrect) #8278
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #8278 +/- ##
=======================================
Coverage 99.10% 99.10%
=======================================
Files 140 140
Lines 4018 4018
=======================================
Hits 3982 3982
Misses 36 36 ☔ View full report in Codecov by Sentry. |
docs/1-general-configuration.md
Outdated
@@ -206,7 +206,7 @@ ActiveAdmin.setup do |config| | |||
config.namespace :admin do |admin| | |||
admin.build_menu :utility_navigation do |menu| | |||
menu.add label: "ActiveAdmin.info", url: "https://www.activeadmin.info", | |||
html_options: { target: :blank } | |||
html_options: { target: :_blank } |
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.
@javierjulio isn't it better to have a string now that this symbol is changing?
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.
Hi @tagliala, I've changed it to use strings instead of symbols
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.
No, it won't matter because it will still covert that to a string. A symbol with a leading underscore is valid. I would have left the format as is but doesn't matter if its a string or not.
e50fcb1
to
e04cb7a
Compare
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.
Usage of quotes does not raise a RuboCop issue but it is not uniform with other quotes in this context
Can you switch to double quotes?
@ndbroadbent can you please rebase and use double quotes instead of single quote? |
e04cb7a
to
42b0d7f
Compare
@tagliala I've rebased and updated the PR, thanks! |
… of target="blank" (incorrect) (#8278) Use target="_blank" instead of target="blank" (incorrect)
Hello,
This is just a small change to some documentation. I noticed that there were some incorrect usages of
target: :blank
, which has also found it's way into some StackOverflow and ChatGPT answers.You can find more information about this here: https://stackoverflow.com/questions/35703005/what-is-the-difference-between-target-blank-and-target-blank
So I just thought it might be worth opening a PR to show users the correct way to do it. Thanks!