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

Onboarding: Remove Language Setting UI #2489

Closed
1 task
Tracked by #2458
joemcgill opened this issue Aug 5, 2024 · 4 comments · Fixed by #2522
Closed
1 task
Tracked by #2458

Onboarding: Remove Language Setting UI #2489

joemcgill opened this issue Aug 5, 2024 · 4 comments · Fixed by #2522

Comments

@joemcgill
Copy link
Collaborator

joemcgill commented Aug 5, 2024

Part of #2458

On the second page of onboarding, the first section a user is shown is the "Audience" section, which includes a "Language" setting that cannot be modified by the user. Remove this setting to reduce unnecessary clutter from this screen.

Before

Image

After

Image

Acceptance Criteria

  • The language section header, description, and control are no longer shown.

Implementation Brief

The page component for the second step of onboarding is the SetupFreeListings component located in js/src/components/free-listings/setup-free-listings/index.js. This page imports FormContent which includes the ChooseAudienceSection component that needs to be edited (in js/src/components/free-listings/choose-audience-section/choose-audience-section.js) to remove the entire subsection that contains the language settings.

Test Coverage

  • Update E2E tests in tests/e2e/specs/setup-mc/step-2-product-listings.test.js .
  • Fix any relevant E2E tests related to the EditFreeCampaign page that may be affected by this change.

Definition Questions

  1. Does the SetupFreeListings component need to be shared still between onboarding and EditFreeCampaign, or should we take this opportunity to clean things up and move this component into the js/src/setup-mc/setup-stepper/ folder? A: this will remain shared.
  2. Is there any back-end infrastructure that will need to be updated or removed if we're no longer showing this setting in the UI of the application?
@eason9487
Copy link
Member

  1. Does the SetupFreeListings component need to be shared still between onboarding and EditFreeCampaign, or should we take this opportunity to clean things up and move this component into the js/src/setup-mc/setup-stepper/ folder?

Could you elaborate on this idea? I don't understand what is to be cleared in this issue's scope, and why it needs to not share the SetupFreeListings component.

As a reference to past changes, it took a lot of time and effort to share them in the past, so there needs to be a clear reason for taking them apart again. See #288

@joemcgill
Copy link
Collaborator Author

Thanks @eason9487. I wasn't sure whether the EditFreeCampaign component was still in use given all of the changes that have been made to these flows, but that link was helpful. I don't see any reason to reorganize the component structure for now.

@mikkamp I'd still be interested in your feedback about whether you know of any backend concerns related to removing this UI, but we can handle in a separate issue if so.

@mikkamp
Copy link
Contributor

mikkamp commented Aug 12, 2024

@mikkamp I'd still be interested in your feedback about whether you know of any backend concerns related to removing this UI, but we can handle in a separate issue if so.

I don't see anything, the language is only returned in the mc/target_audience/suggestions endpoint, but it's never actually set in any of the API calls. Later when products get synced it picks up the site language again. So I think the main purpose of that setting was to show that it was using the site locale for syncing products.

@mikkamp
Copy link
Contributor

mikkamp commented Dec 2, 2024

Closing this as completed since it was part of the 2.9 release.

@mikkamp mikkamp closed this as completed Dec 2, 2024
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 a pull request may close this issue.

8 participants