Skip to content
Snippets Groups Projects

Use GlBroadcastMessage in Broadcast Message admin settings page

What does this MR do and why?

Closes #325352 (closed)

This MR intends to migrate to GlBroadcastMessage as defined in the gitlab-ui and Pajamas Design System.

The component itself was migrated to Rails version in shared/_broadcast_message.html.haml.

The settings page has undergone a breaking change. Background color option was removed. Instead, there's a new option called Theme, which uses available themes from the GlBroadcastMessage component.

The visual design for the banner type has also changed and is now Pajamas-compliant. The notification type was left intact.

The messages configured prior to this change will receive a default indigo theme.

Screenshots or screen recordings

Before After
image image
image image
image image

How to set up and validate locally

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Stanislav Lashmanov

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • thought (non-blocking): Tried testing messages with a target path too, but I ran into a "problem" where they don't show up, this is an unrelated bug since we're not verifying that target_path starts with a / (only the current_path), maybe we should accept paths without a leading /, or disallow this in the UI, or make the format clearer (not for this MR, but probably worth a follow-up)

    [34] pry(main)> BroadcastMessage.current_and_future_messages.banner.second.target_path
    => "admin"
    [35] pry(main)> BroadcastMessage.current_and_future_messages.banner.second.matches_current_path('admin')
    => nil
    
    [36] pry(main)> BroadcastMessage.current_and_future_messages.banner.second.target_path
    => "/admin"
    [37] pry(main)> BroadcastMessage.current_and_future_messages.banner.second.matches_current_path('admin')
    => #<MatchData "/admin">
  • mentioned in issue #357383 (closed)

  • One other question (wasn't saved inline due to #357383 (closed) and had to re-type it): Should we drop the color/font columns in a post-migration if we don't use them anymore? Or do we intend to drop them in a future issue/milestone for the extra "safety"?

  • added 1 commit

    • 4a049ae4 - Use GlBroadcastMessage in Broadcast Message admin settings page

    Compare with previous version

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading