Skip to content

Feat/update layout redirects #22319

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

Open
wants to merge 12 commits into
base: feature/redirection20
Choose a base branch
from

Conversation

kyrylo-polozenko-newfold
Copy link

@kyrylo-polozenko-newfold kyrylo-polozenko-newfold commented May 28, 2025

Context

This PR introduces the initial structure for the Redirections feature in the application. The goal is to establish a foundation for managing and displaying redirections within the admin interface.

Summary

This PR can be summarized in the following changelog entry:

Adds the basic structure and UI for the Redirections page in the WordPress SEO plugin.
Some components were also moved to the shared layer and reused on the settings and redirects pages

UI changes

This PR changes the UI in the plugin. I have added the 'UI change' label to this PR.

Impact check

This PR affects the following parts of the plugin, which may require extra testing:

React frontend structure

@kyrylo-polozenko-newfold kyrylo-polozenko-newfold added UI change PRs that result in a change in the UI changelog: enhancement Needs to be included in the 'Enhancements' category in the changelog labels May 28, 2025
@enricobattocchi enricobattocchi added this to the feature/redirection20 milestone May 29, 2025
@@ -245,9 +245,6 @@ Content.propTypes = {
articleTypeOptions: schemaTypeOptionsPropType.isRequired,
showArticleTypeInput: PropTypes.bool.isRequired,
additionalHelpTextLink: PropTypes.string.isRequired,
helpTextLink: PropTypes.string.isRequired,
helpTextTitle: PropTypes.string.isRequired,
helpTextDescription: PropTypes.string.isRequired,
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of scope

@@ -227,7 +227,7 @@ WincherUpgradeCalloutDescription.propTypes = {
*
* @returns {wp.Element | null} The Wincher upgrade callout.
*/
const WincherUpgradeCallout = ( { onClose, isTitleShortened, trackingInfo } ) => {
const WincherUpgradeCallout = ( { onClose = () => {}, isTitleShortened = false, trackingInfo = {} } ) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of scope

@@ -15,7 +15,7 @@ import TextInput from "../../base/text-input";
*
* @returns {WPElement} A wrapped TextInput for the social inputs.
*/
export default function SocialInput( { id, onChange, socialMedium, isDisabled, ...restProps } ) {
export default function SocialInput( { id = "", onChange = () => {}, socialMedium = "", isDisabled = false, ...restProps } ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of scope, should be in a separate PR.

@enricobattocchi
Copy link
Member

Hey @kyrylo-polozenko-newfold, my teammate @vraja-pro left a first round of comments above
One thing that's not clear to us is that in some files you have added the default values for params for unrelated components, I suppose it was because eslint was complaining? If so, you should probably try to fix any error/warning in the components you are introducing to avoid conflicts - we are in the process to fix the other warnings so there could be some overlap...

@kyrylo-polozenko-newfold
Copy link
Author

Hey @enricobattocchi
yes you are right I made these changes because I had warnings on eslint, I did a rollback

<Button
type="button"
variant="secondary"
className="yst-min-h-[40px]"
Copy link
Contributor

@vraja-pro vraja-pro Jun 3, 2025

Choose a reason for hiding this comment

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

Why min height and why hard coded 40px?

<Button
type="button"
variant="secondary"
className="yst-min-h-[40px]"
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above for class and type

<hr className="yst-absolute yst-inset-x-0 yst-top-1/2 yst-bg-slate-200" />
<button
type="button"
className="yst-relative yst-flex yst-items-center yst-gap-1.5 yst-px-2.5 yst-py-1 yst-mx-auto yst-text-xs yst-font-medium yst-text-slate-700 yst-bg-slate-50 yst-rounded-full yst-border yst-border-slate-300 hover:yst-bg-white hover:yst-text-slate-800 focus:yst-outline-none focus:yst-ring-2 focus:yst-ring-primary-500 focus:yst-ring-offset-2"
Copy link
Contributor

@vraja-pro vraja-pro Jun 3, 2025

Choose a reason for hiding this comment

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

This is a very long line 😅 maybe we can either move it to the style sheet or even better, use existing component in the ui library like the Sidebar navigation.

* @param {string} className The class name.
* @returns {JSX.Element} The element.
*/
export const RouteErrorFallback = ( { className } ) => {
Copy link
Contributor

@vraja-pro vraja-pro Jun 3, 2025

Choose a reason for hiding this comment

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

Please add default if it's not required.

export const RouteErrorFallback = ( { className = "" } ) => {

@vraja-pro vraja-pro self-assigned this Jun 3, 2025
import { useSelectRedirects } from "../hooks";

/**
* @param {string} className The class name.
Copy link
Contributor

Choose a reason for hiding this comment

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

Document optional argument and it's default value.

Suggested change
* @param {string} className The class name.
* @param {string} [className=""] The class name.

description={ __( "Redirect Method desc", "wordpress-seo" ) }
>
<div id="yoast-configuration" className="yst-p-8 yst-max-w-[715px]">
Redirect method
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing translation.

Suggested change
Redirect method
__( "Redirect method", "wordpress-seo" )

description={ __( "Regex Redirects desc", "wordpress-seo" ) }
>
<div id="yoast-configuration" className="yst-p-8 yst-max-w-[715px]">
Regex redirects
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing translation

return <>
<Button
id={ buttonId }
type="button"
Copy link
Contributor

Choose a reason for hiding this comment

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

type button is the default

id={ buttonId }
type="button"
variant="secondary"
className="yst-w-full yst-flex yst-items-center yst-justify-start yst-font-normal yst-text-sm yst-leading-6 yst-text-slate-500 yst-rounded-md yst-border-slate-300 yst-shadow-sm yst-py-1.5 yst-pl-2 yst-pr-3"
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe some of the classes here are already part of the secondary variant and the button component.

<div className="yst-grid yst-grid-cols-3 yst-gap-8 yst-mt-4 yst-items-end">
<div className="yst-flex yst-items-end yst-gap-2 yst-w-full">
<Select
id="bulk-actions"
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
id="bulk-actions"
id="yst-bulk-actions"


<div className="yst-flex yst-items-end yst-gap-2 yst-w-full">
<Select
id="filter-redirect-type"
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
id="filter-redirect-type"
id="yst-filter-redirect-type"

{ ...ariaSvgProps }
/>
<TextField
id="search-redirects"
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
id="search-redirects"
id="yst-search-redirects"

as={ TextField }
type="text"
name="newURL"
id="input-new_url"
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
id="input-new_url"
id="yst-input-new_url"

as={ TextField }
type="text"
name="oldURL"
id="input-old_url"
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
id="input-old_url"
id="yst-input-old_url"

as={ SelectField }
type="select"
name="redirectType"
id="input-redirect_type"
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
id="input-redirect_type"
id="yst-input-redirect_type"

<div className="yst-bg-slate-50 yst-border-slate-200 yst-border-t yst-rounded-b-lg">
<div className="yst-flex yst-align-middle yst-space-x-3 rtl:yst-space-x-reverse yst-p-8">
<Button
id="button-submit-settings"
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
id="button-submit-settings"
id="yst-button-submit-settings"

{ __( "Save changes", "wordpress-seo" ) }
</Button>
<Button
id="button-undo-settings"
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
id="button-undo-settings"
id="yst-button-undo-settings"

export const Search = ( {
buttonId = "button-search",
modalId = "modal-search",
userLocale,
Copy link
Contributor

Choose a reason for hiding this comment

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

If it's not required, we need defaults

Suggested change
userLocale,
userLocale = "en-US",

*
* @param {string} [buttonId="button-search"] Optional ID for the search button element.
* @param {string} [modalId="modal-search"] Optional ID for the modal dialog element.
* @param {string} [userLocale] User's locale used for locale-aware sorting and filtering.
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
* @param {string} [userLocale] User's locale used for locale-aware sorting and filtering.
* @param {string} [userLocale="en-US"] User's locale used for locale-aware sorting and filtering.

* @param {string} [buttonId="button-search"] Optional ID for the search button element.
* @param {string} [modalId="modal-search"] Optional ID for the modal dialog element.
* @param {string} [userLocale] User's locale used for locale-aware sorting and filtering.
* @param {Object[]} queryableSearchIndex Array of searchable items.
Copy link
Contributor

Choose a reason for hiding this comment

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

After adding the typedef above, use it here.

Suggested change
* @param {Object[]} queryableSearchIndex Array of searchable items.
* @param {QueryableSearchIndexItem[]} queryableSearchIndex Array of searchable items.

*/

export const Search = ( {
buttonId = "button-search",
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
buttonId = "button-search",
buttonId = "yst-button-search",

* @param {string} queryableSearchIndex[].fieldId Unique identifier for the item.
* @param {string} queryableSearchIndex[].label Label displayed in the search results.
* @param {string} queryableSearchIndex[].route Route to navigate to when the item is selected.
* @param {string[]} queryableSearchIndex[].keywords Keywords used for matching search queries.
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
* @param {string[]} queryableSearchIndex[].keywords Keywords used for matching search queries.

* @param {string} [modalId="modal-search"] Optional ID for the modal dialog element.
* @param {string} [userLocale] User's locale used for locale-aware sorting and filtering.
* @param {Object[]} queryableSearchIndex Array of searchable items.
* @param {string} queryableSearchIndex[].fieldId Unique identifier for the item.
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
* @param {string} queryableSearchIndex[].fieldId Unique identifier for the item.

* @param {Object[]} queryableSearchIndex Array of searchable items.
* @param {string} queryableSearchIndex[].fieldId Unique identifier for the item.
* @param {string} queryableSearchIndex[].label Label displayed in the search results.
* @param {string} queryableSearchIndex[].route Route to navigate to when the item is selected.
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
* @param {string} queryableSearchIndex[].route Route to navigate to when the item is selected.

* @param {string} [userLocale] User's locale used for locale-aware sorting and filtering.
* @param {Object[]} queryableSearchIndex Array of searchable items.
* @param {string} queryableSearchIndex[].fieldId Unique identifier for the item.
* @param {string} queryableSearchIndex[].label Label displayed in the search results.
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
* @param {string} queryableSearchIndex[].label Label displayed in the search results.

{ map( results, ( groupedItems, index ) => (
<div key={ groupedItems?.[ 0 ]?.route || `group-${ index }` } role="presentation">
<Title
id={ `group-${ index }-title` } as="h4" size="5"
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
id={ `group-${ index }-title` } as="h4" size="5"
id={ `yst-group-${ index }-title` } as="h4" size="5"

strokeLinejoin="round"
d="M7.5 21 3 16.5m0 0L7.5 12M3 16.5h13.5m0-13.5L21 7.5m0 0L16.5 12M21 7.5H7.5"
/>
</svg>
Copy link
Contributor

Choose a reason for hiding this comment

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

You can use <SwitchHorizontalIcon /> from heroicons.

@@ -0,0 +1,94 @@
import { ChildrenLimiter, SidebarNavigation, useSvgAria } from "@yoast/ui-library";
import { useSelectRedirects } from "../hooks";
import { CodeIcon, CogIcon, DownloadIcon } from "@heroicons/react/outline";
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
import { CodeIcon, CogIcon, DownloadIcon } from "@heroicons/react/outline";
import { CodeIcon, CogIcon, DownloadIcon, SwitchHorizontalIcon } from "@heroicons/react/outline";

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: enhancement Needs to be included in the 'Enhancements' category in the changelog UI change PRs that result in a change in the UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants