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.

Sign up for GitHub

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

Improve the usability of the ObjectBrowser when inputting a manual value, checking it on blur, and adding a local validator #6576

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

Improve the usability of the ObjectBrowser when inputting a manual value, checking it on blur, and adding a local validator #6576

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dd8e76f
Improve the usability of the ObjectBrowser when inputting a manual va…
sneridagh Jan 10, 2025
4a22c49
Changelog
sneridagh Jan 10, 2025
98462c1
Simplify error handling. Also make it less confusing
sneridagh Jan 14, 2025
c9d50f5
Update packages/volto/news/6576.bugfix
sneridagh Jan 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/volto/news/6576.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Improve the usability of the `ObjectBrowser` when inputting a manual value, checking it on blur, and adding a local validator. @sneridagh
17 changes: 15 additions & 2 deletions packages/volto/src/components/manage/Widgets/ObjectBrowserWidget.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ import { Image, Label, Popup, Button } from 'semantic-ui-react';
import {
flattenToAppURL,
isInternalURL,
isUrl,
normalizeUrl,
removeProtocol,
} from '@plone/volto/helpers/Url/Url';
import { urlValidator } from '@plone/volto/helpers/FormValidation/validators';
import { searchContent } from '@plone/volto/actions/search/search';
import withObjectBrowser from '@plone/volto/components/manage/Sidebar/ObjectBrowser';
import { defineMessages, injectIntl } from 'react-intl';
Expand Down Expand Up @@ -102,6 +102,7 @@ export class ObjectBrowserWidgetComponent extends Component {
state = {
manualLinkInput: '',
validURL: false,
errors: [],
};

constructor(props) {
Expand Down Expand Up @@ -230,7 +231,16 @@ export class ObjectBrowserWidgetComponent extends Component {

validateManualLink = (url) => {
if (this.props.allowExternals) {
return isUrl(url);
const error = urlValidator({
value: url,
formatMessage: this.props.intl.formatMessage,
});
if (error && url !== '') {
this.setState({ errors: [error] });
} else {
this.setState({ errors: [] });
Copy link
Member

Choose a reason for hiding this comment

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

@sneridagh isn't this redundant due to the initial empty value at line 105?
https://github.com/plone/volto/pull/6576/files#diff-526a68594a32e6dd3a09f1243f7780736303aae2414bb98f8670cbdbc5f71703R105
Technically speaking react should know not to trigger a state chance if it's the same value but I
still think this is unnecessary

Copy link
Member Author

Choose a reason for hiding this comment

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

@ichim-david this is required to reset the state in case that the error is gone.

Copy link
Member Author

Choose a reason for hiding this comment

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

We could reset the state at the top of the method too.

}
return !Boolean(error);
} else {
return isInternalURL(url);
}
Expand Down Expand Up @@ -344,6 +354,8 @@ export class ObjectBrowserWidgetComponent extends Component {
return (
<FormFieldWrapper
{...this.props}
// At the moment, OBW handles its own errors and validation
error={this.state.errors}
className={description ? 'help text' : 'text'}
>
<div
Expand Down Expand Up @@ -372,6 +384,7 @@ export class ObjectBrowserWidgetComponent extends Component {
items.length === 0 &&
this.props.mode !== 'multiple' && (
<input
onBlur={this.onSubmitManualLink}
onKeyDown={this.onKeyDownManualLink}
onChange={this.onManualLinkInput}
value={this.state.manualLinkInput}
Expand Down
Loading