Skip to content

Mass button migration, part 2

Mark Florian requested to merge mf-button-migration-2 into master
ℹ️ UX README
Before diving into the review, pick the files you'll be reviewing and add your name in the UX review tracker below.

What does this MR do and why?

This migrates many links that are styled to look like buttons to the link_button_to helper added in !121772 (merged).

These migrations were largely done blind, in that a codemod was used to perform them.

Note that a number of the migrated link buttons have existing issues, for instance:

  • some do not have an accessible name;
  • some use a disabled attribute/style, which makes no sense for a link.

This MR does not fix those issues. It is simply about replacing link_to calls with CSS classes applied with a better encapsulated link_button_to helper.

Part of https://gitlab.com/gitlab-com/gitlab-OKRs/-/work_items/2030.

Commits

Normalise classes to string

Manually migrate unusual cases

Migrate unusual case

Allow components to use link_button_to

A later commit will update the templates of these components to use this helper method.

Fix previously missed migration

It seems this new RSS link button was added in !123674 (merged) was merged while the previous mass migration MR !121772 (merged) was in review.

Mass button migration

Executed with:

comby \
    -config ~/dev/pajamas-adoption-scanner/comby/button-2.toml \
    -custom-matcher ~/dev/pajamas-adoption-scanner/comby/haml.json \
    -matcher .generic -in-place \
    -f .haml

Using the button-migration branch of the Pajamas Adoption Scanner: https://gitlab.com/gitlab-org/frontend/pajamas-adoption-scanner/-/commits/button-migration

Remove unused styles

Now all buttons here should be Pajamas-compliant, and so colour their icons correctly themselves.

Remove redundant btn-inverse class

Screenshots or screen recordings

In theory, there should be no visual or behavioral differences.

How to set up and validate locally

Because this changes buttons in various places across the app, there's no single process to validate this. Instead, I can provide some heuristics:

  1. Choose a file in the diff.
  2. Based on its name, or the user-facing strings inside it, guess where it appears in the app.
  3. Either:
    • try to perform the actions necessary to render the button(s) in the file, or
    • change the code to force the button to render.
  4. Verify the button looks correct.
  5. Verify clicking the button has the correct behavior. Note: this might not always be a simple page load! Buttons which have, e.g., method: :post or similar on them might have other behavior.

UX review tracker

  1. Once you have picked the files you will review, add your username next to them (or to the containing folder) so that everyone's aware of who is reviewing what, and we avoid duplicate efforts.
  2. After you've reviewed them, check them off. Thank you!
  • 📂 app/
  • 📂 ee/app/
    • components/namespaces/storage/limit_alert_component.html.haml
    • 📂 views/
      • 📂 admin/
        • application_settings/_elasticsearch_form.html.haml
        • licenses/_summary.html.haml
      • 📂 groups/
        • billings/_subgroup_billing_plan_header.html.haml
        • hook_logs/show.html.haml
        • hooks/edit.html.haml @seggenberger
        • protected_environments/_protected_environment.html.haml @danmh
        • saml_group_links/_saml_group_link.html.haml
        • 📂 settings/domain_verification/
          • _lets_encrypt_callout.html.haml
          • new.html.haml
          • show.html.haml
      • 📂 layouts/header/
        • _licensed_user_count_threshold.html.haml
        • _read_only_banner.html.haml
        • _seat_count_alert.html.haml
      • 📂 shared/
        • _manual_quarterly_reconciliation_banner.html.haml
        • _namespace_user_cap_reached_alert.html.haml
        • _new_user_signups_cap_reached_alert.html.haml
        • _submit_license_usage_data_banner.html.haml
        • _ultimate_trial_callout_content.html.haml
        • 📂 credentials_inventory/
          • personal_access_tokens/_personal_access_token.html.haml
          • resource_access_tokens/_resource_access_token.html.haml
        • 📂 promotions/
          • _promote_epics.html.haml
          • _promote_issue_weights.html.haml
          • _promote_mobile_devops.html.haml
          • _promotion_link_project.html.haml
        • web_hooks/_group_web_hook_disabled_alert.html.haml

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 Austin Regnery

Merge request reports