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

update VisibilityCard component #679

Merged
merged 17 commits into from
Sep 3, 2020
Merged

Conversation

AlicjaSzu
Copy link
Contributor

@AlicjaSzu AlicjaSzu commented Aug 26, 2020

I want to merge this change because...

PR intended to be tested with API branch: add-searchable-and-available-for-purchase-flags-to-product

Screenshots

Screenshot 2020-09-02 at 15 14 01

Screenshot 2020-08-27 at 17 13 30

Screenshot 2020-08-28 at 10 34 28

Pull Request Checklist

  1. All visible strings are translated with proper context.
  2. All data-formatting is locale-aware (dates, numbers, and so on).
  3. Translated strings are extracted.
  4. Number of API calls is optimized.
  5. The changes are tested.
  6. Data-test are added for new elements.
  7. Type definitions are up to date.
  8. Changes are mentioned in the changelog.

Test environment config

API_URI=https://add-searchable-and-available-for-purchase-flags-to-product.api.saleor.rocks/graphql/

@patrys
Copy link
Member

patrys commented Aug 26, 2020

@AlicjaSzu AlicjaSzu self-assigned this Aug 27, 2020
@AlicjaSzu AlicjaSzu force-pushed the feature/sale-publication-date branch from 6ba8031 to a0f2287 Compare August 27, 2020 19:15
@github-actions github-actions bot temporarily deployed to feature-sale-publication-date August 27, 2020 19:16 Inactive
@lgtm-com
Copy link

lgtm-com bot commented Aug 27, 2020

This pull request fixes 1 alert when merging a0f2287 into 265c8bc - view on LGTM.com

fixed alerts:

  • 1 for Useless conditional

@github-actions github-actions bot temporarily deployed to feature-sale-publication-date August 27, 2020 20:18 Inactive
@lgtm-com
Copy link

lgtm-com bot commented Aug 27, 2020

This pull request fixes 1 alert when merging 4ee9b90 into 265c8bc - view on LGTM.com

fixed alerts:

  • 1 for Useless conditional

@github-actions github-actions bot temporarily deployed to feature-sale-publication-date August 27, 2020 23:14 Inactive
@lgtm-com
Copy link

lgtm-com bot commented Aug 27, 2020

This pull request fixes 1 alert when merging 865e14b into 265c8bc - view on LGTM.com

fixed alerts:

  • 1 for Useless conditional

@github-actions github-actions bot temporarily deployed to feature-sale-publication-date August 28, 2020 09:33 Inactive
@github-actions github-actions bot temporarily deployed to feature-sale-publication-date August 28, 2020 09:36 Inactive
@lgtm-com
Copy link

lgtm-com bot commented Aug 28, 2020

This pull request fixes 1 alert when merging af5f25a into 265c8bc - view on LGTM.com

fixed alerts:

  • 1 for Useless conditional

@github-actions github-actions bot temporarily deployed to feature-sale-publication-date August 28, 2020 09:50 Inactive
@lgtm-com
Copy link

lgtm-com bot commented Aug 28, 2020

This pull request fixes 1 alert when merging d35b0fa into 265c8bc - view on LGTM.com

fixed alerts:

  • 1 for Useless conditional

@AlicjaSzu AlicjaSzu force-pushed the feature/sale-publication-date branch from d35b0fa to 05cf11c Compare August 31, 2020 07:23
@github-actions github-actions bot temporarily deployed to feature-sale-publication-date August 31, 2020 07:23 Inactive
@lgtm-com
Copy link

lgtm-com bot commented Aug 31, 2020

This pull request fixes 1 alert when merging 05cf11c into b1791b6 - view on LGTM.com

fixed alerts:

  • 1 for Useless conditional

Copy link
Contributor

@dominik-zeglen dominik-zeglen left a comment

Choose a reason for hiding this comment

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

Styling is a little bit off.

Screenshot 2020-08-31 at 12 50 12

src/components/VisibilityCard/VisibilityCard.tsx Outdated Show resolved Hide resolved
src/components/VisibilityCard/VisibilityCard.tsx Outdated Show resolved Hide resolved
src/components/VisibilityCard/VisibilityCard.tsx Outdated Show resolved Hide resolved
@maarcingebala
Copy link
Member

As @dominik-zeglen mentioned there are small styling differences between the screenshot and designs:

  • "Set availability date" and "Set publication date" shouldn't have any underline
  • "Will become available on" and "Since " text have different styles - grey color and smaller line spacing
  • "Disabling this checkbox will remove product ..." - this text is also using a different styling - grey text and smaller line spacing

@github-actions github-actions bot temporarily deployed to feature-sale-publication-date September 2, 2020 12:26 Inactive
@lgtm-com
Copy link

lgtm-com bot commented Sep 2, 2020

This pull request fixes 1 alert when merging 66de625 into f5d3d01 - view on LGTM.com

fixed alerts:

  • 1 for Useless conditional

@github-actions github-actions bot temporarily deployed to feature-sale-publication-date September 2, 2020 12:50 Inactive
@lgtm-com
Copy link

lgtm-com bot commented Sep 2, 2020

This pull request fixes 1 alert when merging a047ce7 into f5d3d01 - view on LGTM.com

fixed alerts:

  • 1 for Useless conditional

@github-actions github-actions bot temporarily deployed to feature-sale-publication-date September 2, 2020 14:38 Inactive
@github-actions github-actions bot temporarily deployed to feature-sale-publication-date September 3, 2020 09:25 Inactive
@github-actions github-actions bot temporarily deployed to feature-sale-publication-date September 3, 2020 09:30 Inactive
@lgtm-com
Copy link

lgtm-com bot commented Sep 3, 2020

This pull request fixes 1 alert when merging 958d66c into dda68cf - view on LGTM.com

fixed alerts:

  • 1 for Useless conditional

@github-actions github-actions bot temporarily deployed to feature-sale-publication-date September 3, 2020 09:41 Inactive
@lgtm-com
Copy link

lgtm-com bot commented Sep 3, 2020

This pull request fixes 1 alert when merging dcd6ee5 into dda68cf - view on LGTM.com

fixed alerts:

  • 1 for Useless conditional

@AlicjaSzu AlicjaSzu merged commit eba4ba2 into master Sep 3, 2020
@AlicjaSzu AlicjaSzu deleted the feature/sale-publication-date branch September 3, 2020 10:25
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.

5 participants