Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

25229 - Remove Learn More button in Business Registry Marketing page #3280

Merged
merged 3 commits into from
Feb 27, 2025

Conversation

ArwenQin
Copy link
Contributor

Issue #:
bcgov/entity#25229

Description of changes:

  • Remove Learn More button in Business Registry Marketing page

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the sbc-auth license (Apache 2.0).

@ArwenQin ArwenQin marked this pull request as ready for review February 27, 2025 18:43
@severinbeauvais
Copy link
Collaborator

/gcbrun

@@ -66,16 +60,9 @@
<script lang="ts">
import { Component, Vue } from 'vue-property-decorator'
import ConfigHelper from '@/util/config-helper'
import LearnMoreButton from '@/components/auth/common/LearnMoreButton.vue'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think this component should be deleted?

Copy link
Contributor Author

@ArwenQin ArwenQin Feb 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's used in other places
image
image

I think these links are needed, right?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh! Good catch.

Yes, please check with Janis.

I assume those other link buttons don't work either?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These links are working and directing to different pages, which seems needed and working well to me.
https://dev.account.bcregistry.gov.bc.ca/request-name

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, it's a generic button with different URLs.

Well, maybe the other instances are OK. Confirm with Janis.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure! Janis is out of office, will add a note and confirm with her next week.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend you merge this PR and create a new ticket if more changes are needed after you chat with Janis.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, could you please merge it? I don't have admin access, thank you!

@severinbeauvais
Copy link
Collaborator

Arwen, please add a comment to your ticket, addressed to @seeker25 , to include it in their next deployment.

@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://bcregistry-account-dev--pr-test-3280-mhafd0ox.web.app

@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://bcregistry-account-dev--pr-3280-b74kvnlh.web.app

@ArwenQin
Copy link
Contributor Author

Test:
https://bcregistry-account-dev--pr-3280-b74kvnlh.web.app/decide-business

Before:
image

Removed:
image

Copy link
Collaborator

@JazzarKarim JazzarKarim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@severinbeauvais severinbeauvais merged commit 379e6c1 into bcgov:main Feb 27, 2025
12 of 13 checks passed
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.

6 participants