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

Add slug field to category, product, collection & page details #720

Merged
merged 38 commits into from
Sep 25, 2020

Conversation

bmigirl
Copy link
Contributor

@bmigirl bmigirl commented Sep 18, 2020

SALEOR-1021

  • Also changed the seo form subtitle to only show when form is collapsed, confirmed with @pwgryglak

✨ After changes:

Screenshot 2020-09-21 at 15 06 31

Screenshot 2020-09-21 at 15 18 00

Screenshot 2020-09-21 at 15 14 29

Pull Request Checklist

  1. Type definitions are up to date.
  2. Changes are mentioned in the changelog.

Test environment config

API_URI=https://master.staging.saleor.rocks/graphql/

@github-actions github-actions bot temporarily deployed to feat-add-slug-to-seo-form September 18, 2020 14:41 Inactive
@github-actions github-actions bot had a problem deploying to storybook feat-add-slug-to-seo-form September 18, 2020 14:41 Failure
@bmigirl bmigirl marked this pull request as draft September 18, 2020 14:42
@bmigirl bmigirl changed the title Feat/add slug to seo form Add slug field to category, product, collection & page details Sep 18, 2020
@patrys
Copy link
Member

patrys commented Sep 18, 2020

@github-actions github-actions bot temporarily deployed to feat-add-slug-to-seo-form September 21, 2020 12:58 Inactive
@github-actions github-actions bot temporarily deployed to storybook feat-add-slug-to-seo-form September 21, 2020 12:58 Inactive
@github-actions github-actions bot temporarily deployed to feat-add-slug-to-seo-form September 21, 2020 13:35 Inactive
@github-actions github-actions bot temporarily deployed to storybook feat-add-slug-to-seo-form September 21, 2020 13:35 Inactive
@github-actions github-actions bot temporarily deployed to feat-add-slug-to-seo-form September 25, 2020 13:39 Inactive
@github-actions github-actions bot temporarily deployed to storybook feat-add-slug-to-seo-form September 25, 2020 13:39 Inactive
@orzechdev orzechdev self-requested a review September 25, 2020 13:43
src/components/SeoForm/SeoForm.tsx Outdated Show resolved Hide resolved
: getPageErrorMessage(error as PageErrorFragment, intl);
};

const handleSlugChange = event => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Untyped function argument

}
};

const getError = fieldName => getFieldError(errors, fieldName);
Copy link
Contributor

Choose a reason for hiding this comment

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

Untyped function argument

@@ -56,6 +56,7 @@ const PageDetailsPage: React.FC<PageDetailsPageProps> = ({
}) => {
const intl = useIntl();
const localizeDate = useDateLocalize();
const pageExists = page === null;
Copy link
Contributor

Choose a reason for hiding this comment

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

pageExists is true if page doesn't exist

"If empty, the preview shows what will be autogenerated."
})}
value={description ? description.slice(0, 299) : undefined}
helperText={intl.formatMessage(seoFieldMessage)}
Copy link
Contributor

Choose a reason for hiding this comment

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

If getError(SeoField.description) === true, this input will be red but won't display any message

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, removing until we get errors for those fields from backend

loading?: boolean;
helperText?: string;
isCreating?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the better name would be allowEmptySlug, it's more understandable from the component's perspective

@github-actions github-actions bot temporarily deployed to feat-add-slug-to-seo-form September 25, 2020 13:57 Inactive
@github-actions github-actions bot temporarily deployed to storybook feat-add-slug-to-seo-form September 25, 2020 13:57 Inactive
@@ -56,6 +56,7 @@ const PageDetailsPage: React.FC<PageDetailsPageProps> = ({
}) => {
const intl = useIntl();
const localizeDate = useDateLocalize();
const pageExists = page === null;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const pageExists = page === null;
const pageNotExists = page === null;

@github-actions github-actions bot temporarily deployed to feat-add-slug-to-seo-form September 25, 2020 14:25 Inactive
@github-actions github-actions bot temporarily deployed to storybook feat-add-slug-to-seo-form September 25, 2020 14:25 Inactive
@bmigirl bmigirl requested review from dominik-zeglen, orzechdev and AlicjaSzu and removed request for AlicjaSzu September 25, 2020 14:26
@github-actions github-actions bot temporarily deployed to feat-add-slug-to-seo-form September 25, 2020 14:45 Inactive
@github-actions github-actions bot temporarily deployed to storybook feat-add-slug-to-seo-form September 25, 2020 14:45 Inactive
@github-actions github-actions bot temporarily deployed to feat-add-slug-to-seo-form September 25, 2020 15:45 Inactive
@github-actions github-actions bot temporarily deployed to storybook feat-add-slug-to-seo-form September 25, 2020 15:45 Inactive
@github-actions github-actions bot temporarily deployed to feat-add-slug-to-seo-form September 25, 2020 16:03 Inactive
@maarcingebala maarcingebala merged commit 7e24e4a into master Sep 25, 2020
@maarcingebala maarcingebala deleted the feat/add-slug-to-seo-form branch September 25, 2020 16:44
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.

9 participants