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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(AMBER-665): Bring PA Artwork Details to parity with designs and mweb #10278

Merged
merged 10 commits into from
May 24, 2024

Conversation

lilyfromseattle
Copy link
Contributor

This PR resolves AMBER-665

Description

This PR brings the mobile artwork page to parity with web and the PA designs. I went over everything with Peter and he confirmed he is happy with the page. I also fixed some small bugs (Read more truncating wrong, price field rendering twice).

Screenshot 2024-05-22 at 1 58 12鈥疨M Screenshot 2024-05-22 at 1 58 05鈥疨M Screenshot 2024-05-22 at 1 56 51鈥疨M

To the reviewers 馃憖

  • I would like at least one of the reviewers to run this PR on the simulator or device.
Changelog updates

Changelog updates

Cross-platform user-facing changes

  • brings mobile artwork page up to parity with web and private artworks design - lily

iOS user-facing changes

Android user-facing changes

Dev changes

Need help with something? Have a look at our docs, or get in touch with us.

@lilyfromseattle lilyfromseattle changed the title Lilyfromseattle/artwork page parity feat(AMBER-665): Bring PA Artwork Details to parity with designs and mweb May 22, 2024
@lilyfromseattle lilyfromseattle requested a review from a team May 22, 2024 19:20
@ArtsyOpenSource
Copy link
Contributor

ArtsyOpenSource commented May 22, 2024

This PR contains the following changes:

  • Cross-platform user-facing changes (brings mobile artwork page up to parity with web and private artworks design - lily)

Generated by 馃毇 dangerJS against d3b0874

})
if (!!artworkBelowTheFold?.isEligibleForArtsyGuarantee) {
sections.push({
key: "artsyGuarantee",
Copy link
Contributor

Choose a reason for hiding this comment

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

Though they shouldnt render at the same time, could there be any clashes here now that we have two keys with the name "artsyGuarantee"?

(I wonder if this is what we are running into in that failing spec? 馃 )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ooooooh just saw this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

don't think it should be an issue as only one is pushed at a time

@@ -557,7 +556,7 @@ describe("Artwork", () => {
})

describe("Artsy Guarantee section", () => {
it("should be displayed when eligible for artsy guarantee", async () => {
fit("should be displayed when eligible for artsy guarantee", async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's remove this fit, I think there are more failures in here!
Maybe they can provide us some clues

@damassi damassi added the Squash On Green A label to indicate that Peril should squash-merge this PR when all statuses are green label May 24, 2024
@lilyfromseattle lilyfromseattle merged commit 8c90390 into main May 24, 2024
6 of 7 checks passed
@lilyfromseattle lilyfromseattle deleted the lilyfromseattle/artwork-page-parity branch May 24, 2024 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Jira Synced Squash On Green A label to indicate that Peril should squash-merge this PR when all statuses are green
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants