NourEldinShobier / carbon-flutter

Flutter UI library based on IBM's Carbon Design System 💎
MIT License
115 stars 9 forks source link

CTextField error border color #3

Closed johnwalicki closed 2 years ago

johnwalicki commented 3 years ago

Hi @NourEldinShobier - We've been successful in using your Carbon-Flutter package (excellent, Thanks!) in the OpenEEW Provisioner app https://github.com/openeew/openeew-provisioner

I'm trying to add some error warnings when the user inputs an incorrect value. How can I change the border outline of CTextField ? I tried adding

        border: new OutlineInputBorder(
           borderSide: new BorderSide(color: CColors.red60)
    )),
NourEldinShobier commented 3 years ago

Hi @johnwalicki ,

To validate user's input, use the validator parameter:

CTextField(
   label: 'Label',
   description: 'Description',
   validator: (value) {
       return CValidationResult(
           type: CValidationResultType.error,
           message: 'Your input is incorrect',
    );
  },
),

I also added examples for CTextField inside lab project (examples directory)

screenshot

Sorry if the project lacks documentation, I promise to document everything after finishing my final exams :v:

Note:

I fixed a bug within the textfield code (in the lab project), so you need to update codebase.

NourEldinShobier commented 3 years ago

After looking at OpenEEW Provisioner source code, I found out that you are using an old version of the package. Currently, the package is being developed in a sub-project called "lab".

In the next hours, I will try migrate the package's code from the lab project to package project. After that you need to:

  1. clone the repo
  2. import the repo (package) into the pubspec.yaml of your project
dependencies:
  carbon:
    path: ../carbon
NourEldinShobier commented 3 years ago

I finished migrating the code. I also added an example project and updated the README to include how to import the package properly into your project.

I will keep the issue open incase there are any problems at any of the mentioned steps.

Also, I have a question: Are there any widgets you need in OpenEEW Provisioner but not included in carbon_flutter yet?

johnwalicki commented 3 years ago

Hi @NourEldinShobier - Thanks for promoting a new version. I will upgrade the OpenEEW Provisioner app.

We have additional design requirements. There is a OpenEEW slack channel for the app. If you're interested, our designer posted screen shot mockups.

I want the CTextField to have a red border and a suffixIcon warning when the WiFi network is incompatible (the ESP32 requires a 2.4GHz network)

johnwalicki commented 3 years ago

I am attempting to migrate to your new carbon-flutter package. flutter run now complains with this error:

lib/main.dart:19:37: Error: The method 'get' isn't defined for the class 'Map<String, Object>'.
 - 'Map' is from 'dart:core'.                                           
 - 'Object' is from 'dart:core'.                                        
Try correcting the name to the name of an existing method, or defining a method named 'get'.
        theme: CarbonThemes.gray100.get<ThemeData>('material-theme'),   
                                    ^^^                                 
lib/main.dart:17:27: Error: The argument type 'Map<String, Object>' can't be assigned to the parameter type 'StyleX'.
 - 'Map' is from 'dart:core'.                                           
 - 'Object' is from 'dart:core'.                                        
 - 'StyleX' is from 'package:stylex/src/style.dart' ('../../../../../walicki/.pub-cache/hosted/pub.dartlang.org/stylex-1.1.0/lib/src/style.dart').
      style: CarbonThemes.gray100,

The main.dart is quite simple: https://github.com/openeew/openeew-provisioner/blob/main/lib/main.dart

NourEldinShobier commented 3 years ago

To change the suffixIcon based on validation, use the icon parameter of the CValidationResult

              CTextField(
                label: 'Label',
                description: 'Description',
                validator: (value) {
                  return CValidationResult(
                    type: CValidationResultType.error,
                    message: 'Your input is incorrect',
                    icon: CSVGIcon.asset('assets/svg/error.svg'),
                  );
                },
              ),

As for the error, stylex is no longer used within the package. You need to remove it from the pubspec.yaml and upgrade flutter_svg to ^0.22.0.

Here is how the main.dart should be:

class OpenEEWProvisioner extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
        title: 'OpenEEW Provisioner',
        home: Start()
     );
  }
}

Note: Currently, the package supports only g100 theme

johnwalicki commented 3 years ago

Closer! I had updated to flutter_svg ^0.22.0 to resolve conflicts already. With your tips, I removed the stylex dependent and fixed up the main.dart.

Now the program is stuck on changes to CFormState. How do I validate the form?

lib/forms/user.dart:34:31: Error: The method 'validate' isn't defined for the class 'CFormState'.
 - 'CFormState' is from 'package:carbon/features/form/form.widget.dart' ('carbon/lib/features/form/form.widget.dart').
Try correcting the name to the name of an existing method, or defining a method named 'validate'.
    if (_formKey.currentState.validate()) {   
NourEldinShobier commented 3 years ago

Form validation is not supported yet, you can only validate each textfield when the input changes.

johnwalicki commented 3 years ago

Ok - let's skip the form validation for now.

My next error is that all of the CTextField objects previously had a this.id, Looks like I can remove the _id variable from the code.

johnwalicki commented 3 years ago

The note RichText object disappeared?

lib/forms/sensor.dart:167:11: Error: No named parameter with the name 'note'.
          note: RichText(                                               

carbon/lib/features/form/form.widget.dart:12:3: Context: Found this candidate, but the arguments don't match.
  CForm({                                                               

lib/forms/provision.dart:99:9: Error: No named parameter with the name 'note'.
        note: RichText(                                                 

carbon/lib/features/form/form.widget.dart:12:3: Context: Found this candidate, but the arguments don't match.
  CForm({                                                               
johnwalicki commented 3 years ago

Thanks for your help porting the OpenEEW Provisioner app to the new Carbon Flutter. I will continue my investigation tomorrow.

NourEldinShobier commented 3 years ago

Can you elaborate more on CTextField id error?

After reviewing my previous commits for CForm, I didn't find the note parameter in any of them. Are you sure that it wasn't added manually to the older version of the package?

johnwalicki commented 3 years ago

The old/original version of CTextField class had a member

class CTextField extends StatefulWidget {
  const CTextField({
    Key key,
    this.id,

I found a downstream commit that added the this.id here: https://github.com/openeew/openeew-provisioner/commit/02039398c8216abe19650120d037044e0464f0c8#diff-7b95719a0c99eb206e543d69dac898f332fdfdea85f71b9fefc44df76fd3c80d

From what I can tell, we don't actually use it in the OpenEEW Provisioner code. I've deleted it for now.

johnwalicki commented 3 years ago

The old/original version of the CForm class had a note member. It is possible that one of our prior contributors added it.

lib/theme/widgets/common/form/form.widget.dart:    this.note,
                if (widget.note != null) ...[
                  const SizedBox(height: 20),
                  widget.note,
                ]

We used it twice in the OpenEEW provisioner app. To keep the app the same, I have added this widget! to the CForm class of the new Carbon library.

lib/forms/provision.dart:        note: RichText(
lib/forms/sensor.dart:          note: RichText(

There's an example of how we use it (the Note: paragraph after the action button) https://github.com/openeew/openeew-provisioner/blob/main/images/OpenEEW-Provisioning-App-Review-Register.png

johnwalicki commented 3 years ago

The last hurdle to compiling the app was to add the validation methods to the CFormState class. I copied a section of code from the original.

It looks like @gdpelican added that in a commit here: https://github.com/openeew/openeew-provisioner/commit/02039398c8216abe19650120d037044e0464f0c8#diff-7b95719a0c99eb206e543d69dac898f332fdfdea85f71b9fefc44df76fd3c80d

If you think it worthy, I can propose a PR to your upstream repo instead of maintaining it as a fork of carbon-theme.

johnwalicki commented 3 years ago

The good news is that I have a working version of the OpenEEW Provisioner App using the new Carbon Theme library. Still a few rough edges. Will clean up a commit and push to the OpenEEW org. We can figure out the slight modifications over upcoming week.

NourEldinShobier commented 3 years ago

I completely agree with the PR, though, using id instead of key is not a good approach, but still I don't have a clear approach in mind because Flutter's FormTextfield doesn't require the key, instead, they wrap the FormTextfield with Formfield. But unlike CTextfield, The FormField validator returns String? instead of CValidationResult.

In summery, I need more time to think about how to do form validation properly :sweat_smile:.

So, let's keep the id member and the validation method as they are in the PR, and later on when I finish form validation, I will mark the id as deprecated to avoid breaking your code.

Carlos-juniorLUKE commented 2 years ago

ola @NourEldinShobier Assim que debug o example aparece esse erro

Error: The method 'enumToString' isn't defined for the class 'CTextFieldState'. package:carbon/…/text_field/text_field.widget.dart:139

NourEldinShobier commented 2 years ago

Hi @Carlos-juniorLUKE

Yes im currently revamping the textfield, it should be fixed within few days

NourEldinShobier commented 2 years ago

Hi @Carlos-juniorLUKE ,

I fixed the text field issue, everything should now work properly

NourEldinShobier commented 2 years ago

Hi @johnwalicki

Regarding the form validation, you can now user GlobalKey with the CFormState to validate the forms' fields as follows:

final formKey = GlobalKey<CFormState>();

formKey.currentState.validate();

the validate method returns a boolean and works as follows:

and sorry if this took ages to be added 🙏 but i was busy with my job

i will close this issue for now unless you have any other questions

johnwalicki commented 2 years ago

Excellent! Thanks for a great library.