Skip to content

fix(links): don't open links in a new window when the flag is unchecked - #267

Open
TheRealAnubis wants to merge 1 commit into
maglevhq:mainfrom
TheRealAnubis:fix/link-open-new-window-cast
Open

TheRealAnubis wants to merge 1 commit into
maglevhq:mainfrom
TheRealAnubis:fix/link-open-new-window-cast

Conversation

@TheRealAnubis

Copy link
Copy Markdown

Links edited in the editor always render with target="_blank", even when "Open in a new window" is unchecked.

Root cause: the URL link form submits the flag through a hidden field (Maglev::Uikit::Form::Link::UrlLinkComponent), so it's stored as a string: "false", or "" when it was never set. Maglev::Content::Link#open_new_window? uses !!link[:open_new_window], which treats any string as true.

Fix:

  • Maglev::Content::Link#open_new_window? casts the value with ActiveModel::Type::Boolean, so links already stored with a string value render correctly.
  • Maglev::SettingTypes::Link#cast_value casts open_new_window to a boolean when the setting is saved (only if the key is present), like the checkbox settings since d5e18e8.

Tests:

  • spec/components/maglev/content/link_spec.rb: open_new_window? / target_blank with "true", "false" and "".
  • spec/models/maglev/setting_types/link_spec.rb: cast_value with "true", "1", "false", "0" and "".
  • Red before the fix (9 failures), green after.

Found while running Maglev 3.1.0 in production, where we've been carrying a monkey-patch for it.

The link form of the editor submits the "Open in a new window" flag
through a hidden field, so it gets stored as a string ("false", or ""
when it was never set). `Maglev::Content::Link#open_new_window?` used
`!!`, which treats those strings as true and renders target="_blank"
on every link edited in the editor.

- cast the flag to a boolean when rendering the link (fixes the links
  already stored with a string value)
- cast it when the link setting is saved, like the checkbox settings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant