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

Remove storyquestions atom #21897

Merged
merged 14 commits into from
Nov 11, 2019
Merged

Remove storyquestions atom #21897

merged 14 commits into from
Nov 11, 2019

Conversation

regiskuckaertz
Copy link
Contributor

Cleaning up after ourselves. The storyquestions atom has been decommissioned.

@regiskuckaertz regiskuckaertz requested a review from a team as a code owner October 9, 2019 14:14
@PRBuilds
Copy link

PRBuilds commented Oct 9, 2019

PRbuilds results:

Screenshots
wide.pngdesktop.pngtablet.pngmobile.png

💚 A11y validation
a11y-report.txt

💚 Microdata Validation
microdata.txt

Apache Benchmark Load Testing
loadtesting.txt

LightHouse Reporting
1573478359.report.html

--automated message

@regiskuckaertz
Copy link
Contributor Author

All looking good now 🎉

@SiAdcock
Copy link
Contributor

Tangentially, we might need to update @AWare's Typescript / Thrift / CAPI unification experiment?

@regiskuckaertz
Copy link
Contributor Author

Yes, I would advise doing so. Can I get a +1? 🙏

@regiskuckaertz regiskuckaertz merged commit 6c18b9e into master Nov 11, 2019
@regiskuckaertz regiskuckaertz deleted the rk-remove-storyquestions branch November 11, 2019 12:50
@prout-bot
Copy link
Collaborator

Overdue on PROD (merged by @regiskuckaertz 30 minutes and 6 seconds ago) What's gone wrong?

@prout-bot
Copy link
Collaborator

Seen on PROD (merged by @regiskuckaertz 3 hours, 25 minutes and 25 seconds ago)

@@ -79,8 +79,6 @@ class AtomPageController(contentApiClient: ContentApiClient, wsClient: WSClient,
lookup(s"atom/$atomType/$id") map {
case Left(model: MediaAtom) =>
renderAtom(MediaAtomPage(model, withJavaScript = isJsEnabled, withVerticalScrollbar = hasVerticalScrollbar))
case Left(model: StoryQuestionsAtom) =>
renderAtom(StoryQuestionsAtomPage(model, withJavaScript = isJsEnabled, withVerticalScrollbar = hasVerticalScrollbar, inApp = inApp))
Copy link
Member

@rtyley rtyley Aug 9, 2022

Choose a reason for hiding this comment

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

Incidentally, this removes the last use of the inApp: Boolean parameter on the render() method, so this can now be removed. The parameter was added with #18213 - we're removing it now with the work in #25190, see 4cb721e.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants