From 79aca4d01504537db68f30292dd5e88e1d5203ff Mon Sep 17 00:00:00 2001 From: Gerardo Navarro Date: Thu, 4 Apr 2024 18:00:59 +0200 Subject: [PATCH 1/2] Protected packages + containers: Adjust style and wording in settings UI - Aligning style and wording in tables `protected packages` and `protected containers` - Improve mobile compatibility - These changes were suggested by @annabeldunstone in https://gitlab.com/gitlab-org/gitlab/-/merge_requests/146523#note_1838085406 - This MR addresses one entry in the implementation plan, see https://gitlab.com/gitlab-org/gitlab/-/issues/413641#checklist-implementation-plan and https://gitlab.com/gitlab-org/gitlab/-/issues/441345#implementation-plan Changelog: other --- .../components/container_protection_rules.vue | 6 +++--- .../packages_protection_rule_form.vue | 4 ++-- .../components/packages_protection_rules.vue | 16 +++++++-------- locale/gitlab.pot | 20 +++++++++---------- .../container_protection_rules_spec.js | 7 ++++--- .../packages_protection_rule_form_spec.js | 6 +++--- .../packages_protection_rules_spec.js | 9 +++++---- 7 files changed, 35 insertions(+), 33 deletions(-) diff --git a/app/assets/javascripts/packages_and_registries/settings/project/components/container_protection_rules.vue b/app/assets/javascripts/packages_and_registries/settings/project/components/container_protection_rules.vue index 4ffd1e6085c9d0..c5296888de647d 100644 --- a/app/assets/javascripts/packages_and_registries/settings/project/components/container_protection_rules.vue +++ b/app/assets/javascripts/packages_and_registries/settings/project/components/container_protection_rules.vue @@ -148,17 +148,17 @@ export default { { key: 'repositoryPathPattern', label: s__('ContainerRegistry|Repository path pattern'), - tdClass: 'gl-w-30 gl-vertical-align-middle!', + tdClass: 'gl-vertical-align-middle!', }, { key: 'pushProtectedUpToAccessLevel', label: I18N_PUSH_PROTECTED_UP_TO_ACCESS_LEVEL, - tdClass: 'gl-w-15 gl-vertical-align-middle!', + tdClass: 'gl-vertical-align-middle!', }, { key: 'deleteProtectedUpToAccessLevel', label: I18N_DELETE_PROTECTED_UP_TO_ACCESS_LEVEL, - tdClass: 'gl-w-15 gl-vertical-align-middle!', + tdClass: 'gl-vertical-align-middle!', }, ], }; diff --git a/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rule_form.vue b/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rule_form.vue index 7c27eacd190ef7..7e3120c478601b 100644 --- a/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rule_form.vue +++ b/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rule_form.vue @@ -122,7 +122,7 @@ export default { diff --git a/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rules.vue b/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rules.vue index 4522b37572d9bf..27b22b9ea73a1e 100644 --- a/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rules.vue +++ b/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rules.vue @@ -42,7 +42,7 @@ export default { }, inject: ['projectPath'], i18n: { - settingBlockTitle: s__('PackageRegistry|Package protection rules'), + settingBlockTitle: s__('PackageRegistry|Protected packages'), settingBlockDescription: s__( 'PackageRegistry|When a package is protected then only certain user roles are able to update and delete the protected package. This helps to avoid tampering with the package.', ), @@ -229,24 +229,24 @@ export default { fields: [ { key: 'col_1_package_name_pattern', - label: s__('PackageRegistry|Package name pattern'), - tdClass: 'gl-w-30', + label: s__('PackageRegistry|Name pattern'), + tdClass: 'gl-vertical-align-middle!', }, { key: 'col_2_package_type', - label: s__('PackageRegistry|Package type'), - tdClass: 'gl-w-10', + label: s__('PackageRegistry|Type'), + tdClass: 'gl-vertical-align-middle!', }, { key: 'col_3_push_protected_up_to_access_level', label: I18N_PUSH_PROTECTED_UP_TO_ACCESS_LEVEL, - tdClass: 'gl-w-15', + tdClass: 'gl-vertical-align-middle!', }, { key: 'col_4_actions', label: '', thClass: 'gl-display-none', - tdClass: 'gl-w-10', + tdClass: 'gl-vertical-align-middle!', }, ], modal: { id: 'delete-package-protection-rule-confirmation-modal' }, @@ -276,7 +276,7 @@ export default { :disabled="isAddProtectionRuleButtonDisabled" @click="showProtectionRuleForm" > - {{ s__('PackageRegistry|Add package protection rule') }} + {{ s__('PackageRegistry|Add protection rule') }} diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 407ce45b2fdc0e..28e974585a24ac 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -35941,7 +35941,7 @@ msgstr "" msgid "PackageRegistry|Add composer registry" msgstr "" -msgid "PackageRegistry|Add package protection rule" +msgid "PackageRegistry|Add protection rule" msgstr "" msgid "PackageRegistry|Additional metadata" @@ -36202,6 +36202,9 @@ msgstr "" msgid "PackageRegistry|Maven XML" msgstr "" +msgid "PackageRegistry|Name pattern" +msgstr "" + msgid "PackageRegistry|Npm" msgstr "" @@ -36240,21 +36243,12 @@ msgid_plural "PackageRegistry|Package has %{updatesCount} archived updates" msgstr[0] "" msgstr[1] "" -msgid "PackageRegistry|Package name pattern" -msgstr "" - msgid "PackageRegistry|Package protection rule deleted." msgstr "" msgid "PackageRegistry|Package protection rule updated." msgstr "" -msgid "PackageRegistry|Package protection rules" -msgstr "" - -msgid "PackageRegistry|Package type" -msgstr "" - msgid "PackageRegistry|Package updated by commit %{link} on branch %{branch}, built by pipeline %{pipeline}, and published to the registry %{datetime}" msgstr "" @@ -36279,6 +36273,9 @@ msgstr "" msgid "PackageRegistry|Project-level" msgstr "" +msgid "PackageRegistry|Protected packages" +msgstr "" + msgid "PackageRegistry|Publish packages if their name or version matches this regex." msgstr "" @@ -36411,6 +36408,9 @@ msgstr "" msgid "PackageRegistry|To widen your search, change or remove the filters above." msgstr "" +msgid "PackageRegistry|Type" +msgstr "" + msgid "PackageRegistry|Unable to fetch package version information." msgstr "" diff --git a/spec/frontend/packages_and_registries/settings/project/settings/components/container_protection_rules_spec.js b/spec/frontend/packages_and_registries/settings/project/settings/components/container_protection_rules_spec.js index 5b95bdc2ea9e5e..1d400f7e76349c 100644 --- a/spec/frontend/packages_and_registries/settings/project/settings/components/container_protection_rules_spec.js +++ b/spec/frontend/packages_and_registries/settings/project/settings/components/container_protection_rules_spec.js @@ -22,7 +22,8 @@ describe('Container protection rules project settings', () => { const $toast = { show: jest.fn() }; const findSettingsBlock = () => wrapper.findComponent(SettingsBlock); - const findTable = () => extendedWrapper(wrapper.findByRole('table', /protected Container/i)); + const findTable = () => + extendedWrapper(wrapper.findByRole('table', { name: /protected containers/i })); const findTableBody = () => extendedWrapper(findTable().findAllByRole('rowgroup').at(1)); const findTableRow = (i) => extendedWrapper(findTableBody().findAllByRole('row').at(i)); const findTableLoadingIcon = () => wrapper.findComponent(GlLoadingIcon); @@ -189,7 +190,7 @@ describe('Container protection rules project settings', () => { .mockResolvedValueOnce(containerProtectionRuleQueryPayload()); const findPaginationButtonPrev = () => - extendedWrapper(findPagination()).findByRole('button', { name: 'Previous' }); + extendedWrapper(findPagination()).findByRole('button', { name: /previous/i }); beforeEach(async () => { createComponent({ containerProtectionRuleQueryResolver }); @@ -228,7 +229,7 @@ describe('Container protection rules project settings', () => { ); const findPaginationButtonNext = () => - extendedWrapper(findPagination()).findByRole('button', { name: 'Next' }); + extendedWrapper(findPagination()).findByRole('button', { name: /next/i }); beforeEach(async () => { createComponent({ containerProtectionRuleQueryResolver }); diff --git a/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rule_form_spec.js b/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rule_form_spec.js index 7697b7f6bd7892..37e31b06704afa 100644 --- a/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rule_form_spec.js +++ b/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rule_form_spec.js @@ -23,8 +23,8 @@ describe('Packages Protection Rule Form', () => { }; const findPackageNamePatternInput = () => - wrapper.findByRole('textbox', { name: /package name pattern/i }); - const findPackageTypeSelect = () => wrapper.findByRole('combobox', { name: /package type/i }); + wrapper.findByRole('textbox', { name: /name pattern/i }); + const findPackageTypeSelect = () => wrapper.findByRole('combobox', { name: /type/i }); const findPushProtectedUpToAccessLevelSelect = () => wrapper.findByRole('combobox', { name: /push protected up to access level/i }); const findSubmitButton = () => wrapper.findByRole('button', { name: /protect/i }); @@ -219,7 +219,7 @@ describe('Packages Protection Rule Form', () => { expect(findAlert().isVisible()).toBe(true); expect(findAlert().text()).toMatch( - /something went wrong while saving the package protection rule/i, + 'Something went wrong while saving the package protection rule', ); }); }); diff --git a/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rules_spec.js b/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rules_spec.js index a7670ef2efc5ca..0264e203d6d134 100644 --- a/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rules_spec.js +++ b/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rules_spec.js @@ -31,14 +31,15 @@ describe('Packages protection rules project settings', () => { const $toast = { show: jest.fn() }; const findSettingsBlock = () => wrapper.findComponent(SettingsBlock); - const findTable = () => extendedWrapper(wrapper.findByRole('table', /protected packages/i)); + const findTable = () => + extendedWrapper(wrapper.findByRole('table', { name: /protected packages/i })); const findTableBody = () => extendedWrapper(findTable().findAllByRole('rowgroup').at(1)); const findTableRow = (i) => extendedWrapper(findTableBody().findAllByRole('row').at(i)); const findTableRowButtonDelete = (i) => findTableRow(i).findByRole('button', { name: /delete/i }); const findTableLoadingIcon = () => wrapper.findComponent(GlLoadingIcon); const findProtectionRuleForm = () => wrapper.findComponent(PackagesProtectionRuleForm); const findAddProtectionRuleButton = () => - wrapper.findByRole('button', { name: /add package protection rule/i }); + wrapper.findByRole('button', { name: /add protection rule/i }); const findAlert = () => wrapper.findByRole('alert'); const findModal = () => wrapper.findComponent(GlModal); @@ -197,7 +198,7 @@ describe('Packages protection rules project settings', () => { .mockResolvedValueOnce(packagesProtectionRuleQueryPayload()); const findPaginationButtonPrev = () => - extendedWrapper(findPagination()).findByRole('button', { name: 'Previous' }); + extendedWrapper(findPagination()).findByRole('button', { name: /previous/i }); beforeEach(async () => { createComponent({ packagesProtectionRuleQueryResolver }); @@ -236,7 +237,7 @@ describe('Packages protection rules project settings', () => { ); const findPaginationButtonNext = () => - extendedWrapper(findPagination()).findByRole('button', { name: 'Next' }); + extendedWrapper(findPagination()).findByRole('button', { name: /next/i }); beforeEach(async () => { createComponent({ packagesProtectionRuleQueryResolver }); -- GitLab From 95b6b124e9dcf59ce76217f8d1a2f0eb115d9238 Mon Sep 17 00:00:00 2001 From: Gerardo Navarro Date: Mon, 8 Apr 2024 18:17:42 +0200 Subject: [PATCH 2/2] refactor: Apply suggestion from @annabeldunstone, @marcel.amirault - Apply small UI styling patch suggested by @annabeldinston, see https://gitlab.com/gitlab-org/gitlab/-/merge_requests/148704#note_1850892748 - Apply the button text "Add rule" as suggested by @marcel.amirault, see https://gitlab.com/gitlab-org/gitlab/-/merge_requests/148704#note_1851173972 --- .../project/components/container_protection_rules.vue | 6 ++---- .../project/components/packages_protection_rule_form.vue | 2 +- .../project/components/packages_protection_rules.vue | 8 +++++--- locale/gitlab.pot | 6 ++++++ .../components/packages_protection_rule_form_spec.js | 2 +- .../settings/components/packages_protection_rules_spec.js | 3 ++- 6 files changed, 17 insertions(+), 10 deletions(-) diff --git a/app/assets/javascripts/packages_and_registries/settings/project/components/container_protection_rules.vue b/app/assets/javascripts/packages_and_registries/settings/project/components/container_protection_rules.vue index c5296888de647d..80e86f52a7149e 100644 --- a/app/assets/javascripts/packages_and_registries/settings/project/components/container_protection_rules.vue +++ b/app/assets/javascripts/packages_and_registries/settings/project/components/container_protection_rules.vue @@ -207,12 +207,10 @@ export default { -
+
{{ __('Protect') }}{{ s__('PackageRegistry|Add rule') }} {{ __('Cancel') }}
diff --git a/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rules.vue b/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rules.vue index 27b22b9ea73a1e..2da970c422f0af 100644 --- a/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rules.vue +++ b/app/assets/javascripts/packages_and_registries/settings/project/components/packages_protection_rules.vue @@ -246,7 +246,7 @@ export default { key: 'col_4_actions', label: '', thClass: 'gl-display-none', - tdClass: 'gl-vertical-align-middle!', + tdClass: 'gl-vertical-align-middle! gl-text-right', }, ], modal: { id: 'delete-package-protection-rule-confirmation-modal' }, @@ -313,6 +313,7 @@ export default { -
+
{ const findPackageTypeSelect = () => wrapper.findByRole('combobox', { name: /type/i }); const findPushProtectedUpToAccessLevelSelect = () => wrapper.findByRole('combobox', { name: /push protected up to access level/i }); - const findSubmitButton = () => wrapper.findByRole('button', { name: /protect/i }); + const findSubmitButton = () => wrapper.findByRole('button', { name: /add rule/i }); const findForm = () => wrapper.findComponent(GlForm); const mountComponent = ({ data, config, provide = defaultProvidedValues } = {}) => { diff --git a/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rules_spec.js b/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rules_spec.js index 0264e203d6d134..d0d414e2c0f329 100644 --- a/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rules_spec.js +++ b/spec/frontend/packages_and_registries/settings/project/settings/components/packages_protection_rules_spec.js @@ -35,7 +35,8 @@ describe('Packages protection rules project settings', () => { extendedWrapper(wrapper.findByRole('table', { name: /protected packages/i })); const findTableBody = () => extendedWrapper(findTable().findAllByRole('rowgroup').at(1)); const findTableRow = (i) => extendedWrapper(findTableBody().findAllByRole('row').at(i)); - const findTableRowButtonDelete = (i) => findTableRow(i).findByRole('button', { name: /delete/i }); + const findTableRowButtonDelete = (i) => + findTableRow(i).findByRole('button', { name: /delete rule/i }); const findTableLoadingIcon = () => wrapper.findComponent(GlLoadingIcon); const findProtectionRuleForm = () => wrapper.findComponent(PackagesProtectionRuleForm); const findAddProtectionRuleButton = () => -- GitLab