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

PHP: Improved validation of rename refactoring #7273

Merged

Conversation

troizet
Copy link
Collaborator

@troizet troizet commented Apr 16, 2024

In rename refactoring, element name validation occurs only when the name is changed, which can lead to a situation like in #6531.

This PR adds element name validation when opening the rename refactoring window.

Before:

before_php_rename_refactoring.mp4

After:

after_php_rename_refactoring.mp4

@troizet troizet added the PHP [ci] enable extra PHP tests (php/php.editor) label Apr 16, 2024
@troizet troizet requested review from tmysik and junichi11 April 16, 2024 11:37
Copy link
Member

@junichi11 junichi11 left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks!

@junichi11 junichi11 added this to the NB22 milestone Apr 16, 2024
Copy link
Member

@tmysik tmysik left a comment

Choose a reason for hiding this comment

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

Thanks for catching that.

@tmysik
Copy link
Member

tmysik commented Apr 16, 2024

Nitpick: Actually, I don't like that we are opening the panel already with an error (it means that the user did not do any change yet but still, there is already an error); personally, I would prefer to have the Preview button disabled (possibly with an info message of type "do your change here").

@troizet
Copy link
Collaborator Author

troizet commented Apr 17, 2024

Nitpick: Actually, I don't like that we are opening the panel already with an error (it means that the user did not do any change yet but still, there is already an error); personally, I would prefer to have the Preview button disabled (possibly with an info message of type "do your change here").

I haven't looked into it much, but it looks like you need to edit the Refactoring API module to do this. Thanks, I'll put it on my task list.

@junichi11 junichi11 merged commit 0090cf9 into apache:master Apr 17, 2024
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PHP [ci] enable extra PHP tests (php/php.editor)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants