Fix: Show NotFound screen when control panel is not found (#7214)
Overview
This PR addresses Issue #7214, which pointed out that visiting an invalid control panel route (e.g., /controlpanel/does-not-exist) currently results in an empty <div />, leaving users with no feedback.
What Changed
- Replaced the empty
<div />fallback with the<NotFound />component inside theControlpanelcomponent. - Now, when an invalid or unknown control panel is accessed, the user is shown the proper "Not Found" page.
Why This Matters
- Improves user experience by clearly signaling routing errors.
- Aligns control panel behavior with the rest of Volto's 404 UX.
Preview
When visiting /controlpanel/non-existent-panel, users will now see:
โ Not Found
Checklist
- [x] Fixes the issue behavior
- [x] Uses existing NotFound component
- [x] Tested in dev environment
- [x] Adds clarity to the UX
Thanks to the maintainers for reviewing! ๐
Closes #7214
@alexandreIFB please review and provide feedback.
Thanks for the review @stevepiercy Iโm working on the suggested changes now:
-Reverting changes to package.json
-Updating changelog with credit
-Checking ReadTheDocs failure
Will push updates shortly.
@shashank0470 please ignore the failing RTD CI checks for now. It's unrelated to this PR, and due to a misconfiguration somewhere that @sneridagh and I need to identify.
Thank you @stevepiercy for pointing that out I was already working on those corrections you mentioned. Honestly, the failing ReadTheDocs CI checks were making me feel a bit hopeless, so I really appreciate the clarification that itโs unrelated to this PR.
All requested changes have been committed. Thanks again @stevepiercy for the helpful feedback! ๐ Let me know if anything else needs adjustment.
I restarted the one failing CI job. Let's see if it fails again.
Actually, the implementation isn't as straightforward as it initially appears. Since the component needs to fetch panel data via API, there's a loading period during which showing NotFound based solely on the absence of data would cause users to incorrectly see the 'not found' screen before the request is even completed.
I managed to reach a satisfactory solution, but faced challenges during testing because routes returning 404 weren't triggering error actions in Redux as expected, making it difficult to validate the correct behavior.
/**
* Controlpanel component.
* @module components/manage/Controlpanels/Controlpanel
*/
import React, { Component } from 'react';
import PropTypes from 'prop-types';
import { connect } from 'react-redux';
import { compose } from 'redux';
import { withRouter } from 'react-router-dom';
import Helmet from '@plone/volto/helpers/Helmet/Helmet';
import {
tryParseJSON,
extractInvariantErrors,
} from '@plone/volto/helpers/FormValidation/FormValidation';
import { createPortal } from 'react-dom';
import { Button, Container, Dimmer, Loader } from 'semantic-ui-react';
import { defineMessages, FormattedMessage, injectIntl } from 'react-intl';
import { toast } from 'react-toastify';
import Icon from '@plone/volto/components/theme/Icon/Icon';
import Toolbar from '@plone/volto/components/manage/Toolbar/Toolbar';
import Toast from '@plone/volto/components/manage/Toast/Toast';
import { Form } from '@plone/volto/components/manage/Form';
import {
updateControlpanel,
getControlpanel,
} from '@plone/volto/actions/controlpanels/controlpanels';
import config from '@plone/volto/registry';
import saveSVG from '@plone/volto/icons/save.svg';
import clearSVG from '@plone/volto/icons/clear.svg';
import NotFound from '@plone/volto/components/theme/NotFound/NotFound';
const messages = defineMessages({
changesSaved: {
id: 'Changes saved.',
defaultMessage: 'Changes saved.',
},
back: {
id: 'Back',
defaultMessage: 'Back',
},
save: {
id: 'Save',
defaultMessage: 'Save',
},
cancel: {
id: 'Cancel',
defaultMessage: 'Cancel',
},
info: {
id: 'Info',
defaultMessage: 'Info',
},
error: {
id: 'Error',
defaultMessage: 'Error',
},
});
/**
* Controlpanel class.
* @class Controlpanel
* @extends Component
*/
class Controlpanel extends Component {
/**
* Property types.
* @property {Object} propTypes Property types.
* @static
*/
static propTypes = {
updateControlpanel: PropTypes.func.isRequired,
getControlpanel: PropTypes.func.isRequired,
id: PropTypes.string.isRequired,
updateRequest: PropTypes.shape({
loading: PropTypes.bool,
loaded: PropTypes.bool,
}).isRequired,
controlpanel: PropTypes.shape({
'@id': PropTypes.string,
data: PropTypes.object,
schema: PropTypes.object,
title: PropTypes.string,
}),
pathname: PropTypes.string.isRequired,
};
/**
* Default properties.
* @property {Object} defaultProps Default properties.
* @static
*/
static defaultProps = {
controlpanel: null,
};
/**
* Constructor
* @method constructor
* @param {Object} props Component properties
* @constructs Controlpanel
*/
constructor(props) {
super(props);
this.onCancel = this.onCancel.bind(this);
this.onSubmit = this.onSubmit.bind(this);
this.state = { isClient: false, error: null };
}
/**
* Component did mount
* @method componentDidMount
* @returns {undefined}
*/
componentDidMount() {
this.props.getControlpanel(this.props.id);
this.setState({ isClient: true });
}
/**
* Component will receive props
* @method componentWillReceiveProps
* @param {Object} nextProps Next properties
* @returns {undefined}
*/
UNSAFE_componentWillReceiveProps(nextProps) {
if (this.props.updateRequest.loading && nextProps.updateRequest.error) {
const message =
nextProps.updateRequest.error?.response?.body?.error?.message ||
nextProps.updateRequest.error?.response?.body?.message ||
nextProps.updateRequest.error?.response?.text ||
'';
const error =
new DOMParser().parseFromString(message, 'text/html')?.all?.[0]
?.textContent || message;
const errorsList = tryParseJSON(error);
let invariantErrors = [];
if (Array.isArray(errorsList)) {
invariantErrors = extractInvariantErrors(errorsList);
}
this.setState({ error: error });
if (invariantErrors.length > 0) {
toast.error(
<Toast
error
title={this.props.intl.formatMessage(messages.error)}
content={invariantErrors.join(' - ')}
/>,
);
}
}
if (this.props.updateRequest.loading && nextProps.updateRequest.loaded) {
toast.info(
<Toast
info
title={this.props.intl.formatMessage(messages.info)}
content={this.props.intl.formatMessage(messages.changesSaved)}
/>,
);
}
}
/**
* Submit handler
* @method onSubmit
* @param {object} data Form data.
* @returns {undefined}
*/
onSubmit(data) {
this.props.updateControlpanel(this.props.controlpanel['@id'], data);
}
/**
* Cancel handler
* @method onCancel
* @returns {undefined}
*/
onCancel() {
this.props.history.goBack();
}
form = React.createRef();
/**
* Render method.
* @method render
* @returns {string} Markup for the component.
*/
render() {
const { filterControlPanelsSchema } = config.settings;
const { controlpanel, getRequest, listRequest } = this.props;
if (
listRequest?.loading ||
getRequest?.loading ||
(!getRequest?.loaded && !getRequest?.error)
) {
return (
<Dimmer active inverted>
<Loader indeterminate size="small">
<FormattedMessage id="loading" defaultMessage="Loading" />
</Loader>
</Dimmer>
);
}
if (!controlpanel) {
return <NotFound />;
}
return (
<div id="page-controlpanel">
<Helmet title={this.props.controlpanel.title} />
<Container>
<Form
ref={this.form}
title={this.props.controlpanel.title}
schema={filterControlPanelsSchema(this.props.controlpanel)}
formData={this.props.controlpanel.data}
requestError={this.state.error}
onSubmit={this.onSubmit}
onCancel={this.onCancel}
hideActions
loading={this.props.updateRequest.loading}
/>
</Container>
{this.state.isClient &&
createPortal(
<Toolbar
pathname={this.props.pathname}
hideDefaultViewButtons
inner={
<>
<Button
id="toolbar-save"
className="save"
aria-label={this.props.intl.formatMessage(messages.save)}
onClick={() => this.form.current.onSubmit()}
disabled={this.props.updateRequest.loading}
loading={this.props.updateRequest.loading}
>
<Icon
name={saveSVG}
className="circled"
size="30px"
title={this.props.intl.formatMessage(messages.save)}
/>
</Button>
<Button
className="cancel"
aria-label={this.props.intl.formatMessage(messages.cancel)}
onClick={() => this.onCancel()}
>
<Icon
name={clearSVG}
className="circled"
size="30px"
title={this.props.intl.formatMessage(messages.cancel)}
/>
</Button>
</>
}
/>,
document.getElementById('toolbar'),
)}
</div>
);
}
}
export default compose(
injectIntl,
connect(
(state, props) => ({
controlpanel: state.controlpanels.controlpanel,
getRequest: state.controlpanels.get,
updateRequest: state.controlpanels.update,
listRequest: state.controlpanels.list,
id: props.match.params.id,
pathname: props.location.pathname,
}),
{ updateControlpanel, getControlpanel },
),
withRouter,
)(Controlpanel);
Additionally, I encountered a routing issue where accessing /controlpanel (singular) first routes through /controlpanels (plural), causing Redux state and loading conflicts.
To address this navigation issue, I had to add a check for listRequest?.loading in the component's loading condition. This prevents the NotFound component from being incorrectly displayed during the period when the component is still loading.
I've implemented all the requested changes. Thanks @stevepiercy and @alexandreIFB for your help and feedback! ๐ Please let me know if there's anything else I should do on my side.
I've implemented all the requested changes. Thanks @stevepiercy and @alexandreIFB for your help and feedback! ๐ Please let me know if there's anything else I should do on my side.
Can't find the changes. Did you forget to push?
@shashank0470 Just a note for next time: itโs important to include a Closes #<issue-number> line in the PR description. Iโve added it to this one, but having it from the start helps keep our workflow organized, links the PR to the issue automatically, and ensures the issue gets closed when the PR is merged.
Thank you for the reminder! @wesleybl Iโll make sure to include a Closes # line in the PR description from next time. Appreciate the guidance โ this helps me follow the workflow better.
Many tips and tricks and other guidance for first-timers is in the aptly named First-time contributors. We're always looking into improve it.