Skip to content

Remove feedback survey on progress view - #53502

Merged
dju90 merged 1 commit into
stagingfrom
remove-progress-feedback-banner
Aug 29, 2023
Merged

Remove feedback survey on progress view#53502
dju90 merged 1 commit into
stagingfrom
remove-progress-feedback-banner

Conversation

@dju90

@dju90 dju90 commented Aug 29, 2023

Copy link
Copy Markdown
Contributor

We have enough (~197) responses on our feedback survey for the progress view, so we're taking it down. Git auto-revert of original PR (#51340) did not work, so doing the removal manually.

Testing story

Tested locally

Before
image

After
image

PR Checklist:

  • Tests provide adequate coverage
  • Privacy and Security impacts have been assessed
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

@dju90
dju90 requested a review from rshipp August 29, 2023 00:50

@hannahbergam hannahbergam left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Happy to approve quickly, just have a quick question!

},
unitSelection: {scriptId: scriptData.id},
locales: {localeCode: 'en-US'},
isRtl: false,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Curious about how this comes into play? I don't see where the banner uses it

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Needed it to pass tests when I first made this change...for some reason 🤔

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Got it- I see now how Notification.jsx uses it (just not passed as a prop). Makes sense that if it wasn't here before the banner, nothing else would need it now that the banner will be one.

@hannahbergam hannahbergam left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@dju90
dju90 merged commit b7e03ff into staging Aug 29, 2023
@dju90
dju90 deleted the remove-progress-feedback-banner branch August 29, 2023 23:32
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.

2 participants