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

Form field validation/error doesn't highlight the tab where the error occurred #2944

Closed
2 tasks done
mark-a-j-adriano opened this issue May 21, 2024 · 2 comments
Closed
2 tasks done

Comments

@mark-a-j-adriano
Copy link

mark-a-j-adriano commented May 21, 2024

Module version(s) affected

5.2.1

Description

An EmailField validating the input does not cascade into a Page Error.

Screenshot 2024-05-21 at 10 58 45 AM

As you can see below when the Field is required then it will show a Tab notification icon and a page error on top

Screenshot 2024-05-21 at 10 58 30 AM

More context: https://silverstripe-users.slack.com/archives/CCP2NDQTX/p1716246349181239

AC:

How to reproduce

<?php

namespace ThisIsASample\Page;

use Page;
use SilverStripe\Forms\CompositeValidator;
use SilverStripe\Forms\EmailField;
use SilverStripe\Forms\RequiredFields;

class SamplePage extends Page
{
    private static string $table_name = 'Samples';

    private static array $db = [
        'Title' => 'Varchar',
        'Email1'  => 'Varchar',
    ];

    private static bool $show_in_sitetree = true;

    private static bool $can_be_root = true;

    /**
     * Returns a FieldList with which to create the main editing form.
     *
     * @return FieldList
     */
    public function getCMSFields()
    {
        $fields = parent::getCMSFields();

        $fields->addFieldToTab('Root.SampleTab', EmailField::create('Email1'));

        return $fields;
    }

    public function getCMSCompositeValidator(): CompositeValidator
    {
        $validator = parent::getCMSCompositeValidator();
        $validator->addValidator(RequiredFields::create([
            'Email1',
        ]));
        return $validator;
    }
}

Possible Solution

No response

Additional Context

No response

Validations

  • Check that there isn't already an issue that reports the same bug
  • Double check that your reproduction steps work in a fresh installation of silverstripe/installer (with any code examples you've provided)

PRs

@GuySartorelli
Copy link
Member

GuySartorelli commented May 21, 2024

Thanks for raising this. It's definitely a bug.

I'm not sure what you mean by "page error". We have two validation error messages here:

  1. EmailField form field validation error message - this is a form validation error
  2. RequiredFields validator error message - this is a form validation error

The only difference I can see between the two with a quick glance is that one uses the "required" message type, and the other uses the "validation" message type - but both should definitely display the same way.

I would expect a "page error" to be one that comes from the validate() method on your page. Did you check whether that one indicates the tab or not?

You've raised this against the silverstripe/silverstripe-cms repository - does this mean you have confirmed that these both behave/display the same way when editing records in a GridField (e.g. in a ModelAdmin)?

@GuySartorelli GuySartorelli changed the title Form field validation/error does not cascade to Page validation/error Form field validation/error doesn't highlight the tab where the error occurred May 21, 2024
@GuySartorelli GuySartorelli self-assigned this May 22, 2024
@GuySartorelli
Copy link
Member

Turns out the gridfield edit form doesn't have the tab highlight functionality, so this is specific to editing SiteTree.
Confirmed that this is only incorrect when using validation from form fields, which for some reason use "validation" as the error type, which isn't technically a valid type (should use one of the consts on ValidationResult), but it's been there forever so we can't just change that in a patch.

I'll make a PR with the minimal change required to fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants