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

Apply new input validation method for Create Plugin dialog #76778

Merged
merged 1 commit into from
Aug 11, 2023

Conversation

eugene87222
Copy link
Contributor

@eugene87222 eugene87222 commented May 6, 2023

Fix #76681

  • Add status panel to the create plugin dialog
  • Remove unused info icons (for old validation)
Old New

@YeldhamDev YeldhamDev added this to the 4.1 milestone May 7, 2023
@eugene87222 eugene87222 force-pushed the create-plugin-dialog branch 2 times, most recently from 7ab802d to aa28f0d Compare May 7, 2023 15:51
Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

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

I left some style comments that need to be addressed, but otherwise looks good.

I think given how this validation convention is widely used now, it might be worth it to create a dedicated class to handle it. I have some idea, but it's for another PR.

@akien-mga akien-mga modified the milestones: 4.1, 4.2 Jun 14, 2023
@YuriSizov
Copy link
Contributor

Is this an overall improvement though? We lose direct association between the error (or successful validation) and the field.

@KoBeWi
Copy link
Member

KoBeWi commented Jul 12, 2023

Well, you could say the same about every dialog where we use this type of validation.

@YuriSizov
Copy link
Contributor

Yes, you could. Which makes me skeptical about it in general. In some more complex cases it kind of makes sense. But here it's a simple form, and we already have nice validation, at least as far as immediate hints from icons are concerned. And we want to remove that for some text that you need to read and figure out and associate back with the form.

@KoBeWi
Copy link
Member

KoBeWi commented Jul 12, 2023

#78744 will unify all these validations into a single class, which will allow us to easily modify it. Then we can e.g. associate specific errors with some fields.

@KoBeWi
Copy link
Member

KoBeWi commented Aug 8, 2023

#78744 was merged, so this needs to be updated. It will simplify the code.

@KoBeWi
Copy link
Member

KoBeWi commented Aug 8, 2023

The "subfolder valid" message still appears when you edit the plugin, even though the path field is not visible. I think the path message should not be displayed when editing.

@akien-mga akien-mga merged commit a22cadf into godotengine:master Aug 11, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

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

Successfully merging this pull request may close these issues.

Create Plugin dialog's validation should be changed for consistency
5 participants