Ux buttons - Deprecating the classes btn-success, btn-info, and btn-warning

[bisht-richa]

What does this PR change?

This PR partially fixes #24324 - Deprecating the classes btn-success, btn-info, btn-link, and btn-warning.

• Removed the btn-success class from CSS and pages
• Removed the btn-info class from CSS and pages
• Removed the btn-warning class from CSS and page
• Removed the btn-link class from some pages. (Some instances still remain and will be addressed in future updates.)
• Modify <rhn:toolbar> component - ToolbarTag.java file
  • Added the btn-primary CSS class to createType button,
  • Applied the btn-default CSS classe to the updateType and cloneType buttons.
  • Applied the btn-danger CSS class to the deleteType button.
  • Modify the code to ensure that the createType button is always positioned at the far right,
• Modify <FilterEdit> component
  • Replaced ModalLink> with <ModalButton> component to launch a modal dialog
  • Added className props

GUI diff

No difference.

Before:

After:

• DONE

Documentation

• No documentation needed

• DONE

Test coverage

ℹ️ If a major new functionality is added, it is strongly recommended that tests for the new functionality are added to the Cucumber test suite

• No tests: already covered

• DONE

Links

Fixes: #25737
Issue(s): #24324
Port(s): # add downstream PR(s), if any

• DONE

Changelogs

Make sure the changelogs entries you are adding are compliant with https://github.com/uyuni-project/uyuni/wiki/Contributing#changelogs and https://github.com/uyuni-project/uyuni/wiki/Contributing#uyuni-projectuyuni-repository

If you don't need a changelog check, please mark this checkbox:

• No changelog needed

If you uncheck the checkbox after the PR is created, you will need to re-run changelog_test (see below)

Re-run a test

If you need to re-run a test, please mark the related checkbox, it will be unchecked automatically once it has re-run:

• Re-run test "changelog_test"
• Re-run test "backend_unittests_pgsql"
• Re-run test "java_pgsql_tests"
• Re-run test "schema_migration_test_pgsql"
• Re-run test "susemanager_unittests"
• Re-run test "javascript_lint"
• Re-run test "spacecmd_unittests"

Before you merge

Check How to branch and merge properly!

[github-actions]

👋 Hello! Thanks for contributing to our project.
Acceptance tests will take some time (aprox. 1h), please be patient ☕
You can see the progress at the end of this page and at https://github.com/uyuni-project/uyuni/pull/9368/checks
Once tests finish, if they fail, you can check 👀 the cucumber report. See the link at the output of the action.
You can also check the artifacts section, which contains the logs at https://github.com/uyuni-project/uyuni/pull/9368/checks.

If you are unsure the failing tests are related to your code, you can check the "reference jobs". These are jobs that run on a scheduled time with code from master. If they fail for the same reason as your build, it means the tests or the infrastructure are broken. If they do not fail, but yours do, it means it is related to your code.

Reference tests:

• https://github.com/uyuni-project/uyuni/actions/workflows/acceptance_tests_secondary_parallel.yml?query=event%3Aschedule

• https://github.com/uyuni-project/uyuni/actions/workflows/acceptance_tests_secondary.yml?query=event%3Aschedule

KNOWN ISSUES

Sometimes the build can fail when pulling new jar files from download.opensuse.org . This is a known limitation. Given this happens rarely, when it does, all you need to do is rerun the test. Sorry for the inconvenience.

For more tips on troubleshooting, see the troubleshooting guide.

Happy hacking!
⚠️ You should not merge if acceptance tests fail to pass. ⚠️

[github-actions]

Suggested tests to cover this Pull Request

• allcli_overview_systems_details
• min_cve_id_new_syntax
• proxy_container_branch_network
• min_deblike_salt_install_package
• srv_salt
• sle_minion
• srv_power_management_redfish
• buildhost_docker_auth_registry
• min_salt_minion_details
• min_deblike_ssh
• min_rhlike_ssh
• min_deblike_openscap_audit
• srv_channel_api
• srv_rename_hostname
• srv_delete_channel_from_ui
• min_move_from_and_to_proxy
• srv_patches_page
• srv_user_configuration_salt_states
• proxy_container_retail_mass_import
• proxy_traditional_cobbler_pxeboot
• min_salt_formulas_advanced
• srv_dist_channel_mapping
• srv_reportdb
• srv_group_union_intersection
• srv_advanced_search
• srv_docker_cve_audit
• min_rhlike_salt
• srv_mainpage
• srv_delete_channel_with_tool
• minssh_tunnel
• srv_enable_sync_products
• min_cve_audit
• min_salt_minions_page
• min_activationkey
• min_ansible_control_node
• srv_salt_download_endpoint
• min_salt_formulas
• min_bootstrap_ssh_key
• srv_power_management_api
• srv_cobbler_distro
• min_rhlike_salt_install_package_and_patch
• srv_scc_user_credentials
• allcli_update_activationkeys
• min_empty_system_profiles
• buildhost_osimage_build_image
• min_timezone
• allcli_action_chain
• srv_restart
• min_deblike_salt_install_with_staging
• proxy_container_cobbler_pxeboot
• srv_cobbler_profile
• srv_handle_config_channels_with_ISS_v2
• min_rhlike_remote_command
• min_recurring_action
• srv_logfile
• min_monitoring
• minssh_salt_install_package
• srv_task_status_engine
• srv_cobbler_buildiso
• srv_content_lifecycle
• proxy_retail_pxeboot_and_mass_import
• min_custom_pkg_download_endpoint
• min_action_chain
• srv_change_task_schedule
• min_deblike_monitoring
• min_change_software_channel
• min_retracted_patches
• srv_notifications
• allcli_config_channel
• min_config_state_channel_subscriptions
• srv_docker_advanced_content_management
• srv_monitoring
• min_config_state_channel
• srv_add_rocky8_repositories
• min_docker_api
• proxy_container_retail_pxeboot
• min_rhlike_monitoring
• srv_maintenance_windows
• min_bootstrap_api
• srv_manage_channels_page
• min_deblike_salt
• min_salt_pkgset_beacon
• minkvm_guests
• allcli_software_channels
• srv_payg_ssh_connection
• minssh_action_chain
• buildhost_docker_build_image
• srv_menu
• min_rhlike_openscap_audit
• srv_datepicker
• srv_handle_software_channels_with_ISS_v2
• min_check_patches_install
• srv_clone_channel_npn
• min_salt_user_states
• srv_check_channels_page
• min_virthost
• min_salt_lock_packages
• min_salt_install_package
• srv_sync_channels
• allcli_system_group
• srv_distro_cobbler
• min_project_lotus
• srv_cobbler_sync
• min_bootstrap_negative
• min_salt_openscap_audit
• sle_ssh_minion
• min_salt_migration
• srv_change_password
• srv_check_sync_source_packages
• allcli_reboot
• allcli_software_channels_dependencies
• proxy_branch_network
• min_salt_software_states
• srv_users
• srv_power_management
• buildhost_bootstrap
• proxy_traditional_retail_pxeboot
• min_ssh_tunnel
• proxy_as_pod_basic_tests
• srv_check_reposync
• min_bootstrap_reactivation
• minssh_bootstrap_api
• srv_activationkey_api
• min_salt_mgrcompat_state
• srv_push_package
• min_salt_install_with_staging
• minssh_move_from_and_to_proxy
• min_deblike_remote_command
• proxy_traditional_retail_mass_import
• min_bootstrap_script
• srv_sync_fake_channels
• proxy_container
• srv_first_settings
• srv_disable_local_repos_off
• srv_create_repository
• srv_create_fake_channels
• srv_user_preferences
• srv_disable_scheduled_reposync
• allcli_sanity
• srv_virtual_host_manager
• srv_create_activationkey
• srv_organization_credentials
• proxy_traditional
• srv_create_fake_repositories
• srv_sync_products
• proxy_register_as_minion_with_script
• minssh_ansible_control_node
• proxy_cobbler_pxeboot
• srv_channels_add
• srv_docker
• srv_wait_for_reposync
• srv_osimage
• srv_custom_system_info
• srv_manage_activationkey

[Etheryte]

Here and below, I think we need different descriptions for the changelog. While this describes what's changed from the technical side, the changelog is for customers, not engineers. For a customer, things like btn-success etc don't say anything meaningful. I think something along the lines of "Updated user interfaces for pages X, Y, Z" or something similar would work.

Maybe @deneb-alpha has a good suggestion, you're usually pretty good with changelog entries?

[bisht-richa]

@deneb-alpha I have updated the change log description. Please let me know if it needs more improvement.

[Etheryte]

I think this is a very welcome change, we have too many different buttons and they're used in a variety of ways, so simplifying all of that is very nice. One thing to consider once we're done with the cleanup is that we might want to revisit some of the names afterwards. Right now we have for example class="btn" and class="btn btn-default", which can be confusing as a name, as you would think that default is what you get out of the box. There's historical context here of course, but maybe once we're done with this, we can think of a better name?

In general though, this looks good, there were a few nitpicks here and there, but nothing major, this looks good. 👍

[Etheryte]

Here and in the rest of this file, the formatting is wrong. It seems whatever autoformatter you're using doesn't recognize that these are references, not actual opening tags.

@cbbayburt Do you happen to know if we can autofix formatting in jsp files as well somehow conveniently? Would be nice to just have a linter do all of this automatically.

[Etheryte]

This file is safe to drop, we don't include or build .less files anymore since we dropped the old theme. The .scss files are now the only source of styles.

[Etheryte]

This file can be deleted, I'm guessing this got mixed up when you rebased the branch.

[Etheryte]

Since we now only allow a narrow set of class names etc, I would recommend updating the prop type definitions in the button component. E.g. instead of foo?: string, I think we should now have foo?: "btn-primary" | "..." and list all allowed values. That way no one can go and use the wrong values again.

[Etheryte]

This looks good, thanks a lot for putting what must've been a massive effort into this.

Two things we need to do before we merge this: one, ensure we remove the .less files that I mentioned above, none should now exist, and two, fix the jsp formatting.

Outside of that this looks very nice. 👍

When merging this, I would recommend squashing, since that way it's easier to reason about this commit later.

[bisht-richa]

Since we now only allow a narrow set of class names etc, I would recommend updating the prop type definitions in the button component. E.g. instead of foo?: string, I think we should now have foo?: "btn-primary" | "..." and list all allowed values. That way no one can go and use the wrong values again.

I suggest fixing this use case in a separate PR which we can use to resolve other issues/modifications needed in the Button component.

[deneb-alpha]

From releng side, LGTM! thanks