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

fix: address new save and follow issues #11102

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

dariakoko
Copy link
Contributor

@dariakoko dariakoko commented Nov 15, 2024

This PR resolves []

Description

Followup on #11062
Address misalignments found during the QA session:

Artwork card adjustments Activity item adjustments
image image

PR Checklist

  • I have tested my changes on iOS and Android.
  • I hid my changes behind a feature flag, or they don't need one.
  • I have included screenshots or videos, or I have not changed the UI.
  • I have added tests, or my changes don't require any.
  • I added an app state migration, or my changes do not require one.
  • I have documented any follow-up work that this PR will require, or it does not require any.
  • I have added a changelog entry below, or my changes do not require one.

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

  • address new save and follow issues found during the QA - daria

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.

@dariakoko dariakoko self-assigned this Nov 15, 2024
@dariakoko dariakoko marked this pull request as ready for review November 18, 2024 12:14
@ArtsyOpenSource
Copy link
Contributor

ArtsyOpenSource commented Nov 18, 2024

This PR contains the following changes:

  • Cross-platform user-facing changes (address new save and follow issues found during the QA - daria - dariakoko)

Generated by 🚫 dangerJS against 095a443

olerichter00
olerichter00 previously approved these changes Nov 18, 2024
Copy link
Contributor

@olerichter00 olerichter00 left a comment

Choose a reason for hiding this comment

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

Thanks 🌟

}

export const ArtworkItemCTAs: React.FC<ArtworkItemCTAsProps> = ({
artwork: artworkProp,
showSaveIcon = false,
// show follow icon by default, but allow it to be hidden on specific grids
Copy link
Contributor

Choose a reason for hiding this comment

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

Always better to use a JSDoc comment. Like this, it's visible in editors like VSCode.

Suggested change
// show follow icon by default, but allow it to be hidden on specific grids
/**
* Show follow icon by default, but allow it to be hidden on specific grids
*/

Bildschirmfoto 2024-11-18 um 13 59 35

MounirDhahri
MounirDhahri previously approved these changes Nov 18, 2024
@dariakoko dariakoko added the Squash On Green A label to indicate that Peril should squash-merge this PR when all statuses are green label Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

4 participants