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

[SuperTextField] New variations of AttributedTextEditingController.clear() (Resolves #2094) #2125

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

Conversation

CillianMyles
Copy link
Member

@CillianMyles CillianMyles commented Jun 24, 2024

(Resolves #2094)

@@ -803,7 +803,192 @@ void main() {
});
});

group("clearText", () {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we've tried to move away from naming groups or tests after a specific method. Can you describe the group and tests in terms of regular English? That terminology can still use words like "clear" and "text" but should read as a series of words.

You can also using a trailing ">" in group names, where appropriate. If you check Super Editor, you'll see a lot of groups named like that.

@@ -803,7 +803,192 @@ void main() {
});
});

group("clearText", () {
test('clears the text', () {
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like there are separate tests from simultaneous expected conditions. There probalby isn't any reason to separate them. It looks like you should probably verify all conditions in the same test. You'll just want to make sure the test name captures the goal.

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

Successfully merging this pull request may close these issues.

[BUG] - AttributedTextEditingController.clear() not working as expected
2 participants