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

Improvement : Color customization #18

Open
VuillaumeGautier opened this issue Sep 3, 2021 · 7 comments
Open

Improvement : Color customization #18

VuillaumeGautier opened this issue Sep 3, 2021 · 7 comments

Comments

@VuillaumeGautier
Copy link

Hi !
I'd like to suggest an improvement regarding the color customization. Some of them can be coded in the configuration, while others are obtained through the MaterialApp, and some are hard coded.

Putting each of them in the ImagePickerConfigs would be a great upgrade.

Thanks for the work as always, the lib is awesome !

@rydmike
Copy link
Contributor

rydmike commented Sep 3, 2021

I agree! 👍🏻

@weta-vn this was going to be one of my suggestions too. All colors (and text styles too btw) should get defaults from the theme, but be adjustable too in the config. I did notice before when I made the PR for AppBar in the resizing tool that some were hard coded or missing like mentioned in this issue, but at the time the defaults were OK for me, but I was thinking the same that they need to be included as well.

Now that you did the big linting migration and setup #15, I could make some PRs to assist with this issue relatively easy. I can just do it, while I out of curiosity look at the code with the lints applied. If I make some fixes for this, while checking out the new linted code, I will submit a PR referencing this issue.

@weta-vn
Copy link
Owner

weta-vn commented Sep 3, 2021

@VuillaumeGautier @rydmike
Thanks for your suggestion.
We'll try to improve in the next work.

@rydmike
Copy link
Contributor

rydmike commented Sep 25, 2021

@VuillaumeGautier could you give some specifics about which color configuration options you would like to see added?

There are quite a large number of them that can be changed already, but yes not all.

I just made a rather large PR, but I did not add any now color customization yet, but I could. It would help if you could list which elements you in particular would like to change color on that you cannot change now.

@VuillaumeGautier
Copy link
Author

@rydmike Hi ! The color to change are about the Appbar : the icons themselves, its color, the color of the bar and its text, the validation button color while active or inactive and the text color for each case.
As you said before, the best would be for everything to be configurable, but the appbar is the main thing. What comes in my mind is the different icons through the lib, and the dialog when leaving it.

@rydmike
Copy link
Contributor

rydmike commented Sep 27, 2021

You should be able to change most of those colors already now, by wrapping the picker in a custom Theme, if the widgets use their Flutter framework default color behavior you can change them that way.

Of course, if you use theming correctly in your app (with that I mean the way the Flutter apps are designed to be themed according to how it is made), then most colors in the picker are probably as you want them already, since they would then already be following your themed application's colors.

But if you want the picker to deviate from your app theme, then yes sure it is more convenient if the properties are available directly, instead of wrapping it with its own theme to change them, no argument there.

I will make some tests to see what currently actually works when you wrap with a Theme, and post an example here of how to do it.

@VuillaumeGautier
Copy link
Author

@rydmike Definitly, most of them were possible to change, either by theme or by the config, but it was not centralized and some colors were hardcoded (I haven't checked since), like the grey color of the validation button.
About the theme, I'm still reworking a lot of the app and the theme itself which was badly used, so i cannot guarantee the theming works properly for us (I'm thinking of the dialog which should be theme defined).
Thanks for the hard work :)

@rydmike
Copy link
Contributor

rydmike commented Sep 27, 2021

Hi again @VuillaumeGautier,

I totally agree, it can certainly be improved. The other day I submitted a PR that contains some new features regarding the Done button as well.

It can now use a normal IconButton instead, that is typically used in an AppBar and instead of being disabled, you can choose to hide it instead when nothing has been selected.

You can also start the picker in another flash mode than auto flash, and optionally hide the flash button if you eg want to start in OFF, and not even allow users to modify it all at all.

You can also hide the camera rotation button, instead of having it be disabled if you only enabled front or back camera.

Here is the promised example of some theming with Theme wrapper, just to try it out and to demo the new features:

  /// Picks a list of images, either from the gallery or the camera.
  static Future<List<ImageObject>> pickImage(
    final BuildContext context, {
    required final int maxImages,
  }) async {
    // Setup image picker camera configuration.
    final ImagePickerConfigs configs = ImagePickerConfigs();
    // Disable adding stickers feature, we do not want it, the other
    // image editing features are OK.
    configs.stickerFeatureEnabled = false;
    configs.albumThumbHeight = 250;
    configs.thumbHeight = 90;
    // Start with flash OFF and show flash settings button.
    configs.flashMode = FlashMode.off;
    configs.showFlashMode = true;
    // Only back camera used, and show flip button, it will be disabled when only one camera is used
    configs.cameraLensDirection = 1;
    configs.showLensDirection = true;
    // Image size and quality settings.
    configs.maxWidth = ImageHelper.maxWidth;
    configs.maxHeight = ImageHelper.maxHeight;
    configs.compressQuality = ImageHelper.compressQuality;
    // Show an alert when no image was selected when returning.
    configs.showNonSelectedAlert = true;
    // Show an alert when removing selected images.
    configs.showRemoveImageAlert = true;
    // Done button style, use default outlined button style and disabled when nothing selected.
    configs.doneButtonStyle = DoneButtonStyle.outlinedButton;    
    configs.doneButtonDisabledBehavior = DoneButtonDisabledBehavior.disabled;
    // Translate function, just defaults and a dummy translate for now that are
    // used to change the default labels.
    configs.translateFunc = _dummyTranslate;

    final List<ImageObject>? images = await Navigator.of(context).push(
      PageRouteBuilder(
          // Use fullscreen dialog, we get a X close icon, instead of <-
          fullscreenDialog: true,
          pageBuilder: (final BuildContext context,
              final Animation<double> animation, __) {
            return AnnotatedRegion<SystemUiOverlayStyle>(
              value: FlexColorScheme.themedSystemNavigationBar(context),
              child: Theme(
                data: Theme.of(context).copyWith(
                    appBarTheme: const AppBarTheme(
                      backgroundColor: Colors.black,
                      foregroundColor: Colors.white,
                    ),
                    dialogBackgroundColor: Colors.blue[100],
                    outlinedButtonTheme: OutlinedButtonThemeData(
                        style: OutlinedButton.styleFrom(
                      padding: EdgeInsets.zero,
                    ))),
                child: ImagePicker(
                  maxCount: maxImages > ImageHelper.maxImages
                      ? ImageHelper.maxImages
                      : maxImages,
                  isCaptureFirst: true,
                ),
              ),
            );
          }),
    );
    return images ?? <ImageObject>[];
  }
}

// With this function we override or hide some of the default labels in the camera.
// With the new PR the row for 'image_picker_select_images_guide' is completely removed when blank
// and the 'image_picker_select_images_title' is blank there is no colon and space added.
String _dummyTranslate(final String name, final String value) {
  if (name == 'image_picker_select_images_title') return '';
  if (name == 'image_picker_select_images_guide') return '';
  if (name == 'image_picker_select_button_title') return 'DONE';
  return value;
}

Above we make AppBar black and text and icons in it white, and we add a crazy blue dialog background. The OutlinedButton theme was just added because the surrounding app theme defines an OutlinedButton theme with padding that makes it not fit at all in an AppBar, so it was removed here for this demo, useful as demo too I guess.

With it we get something like below. With hard coded grey disabled done button, personally I think it should just follow theme default disabled style for the outlined button, then users can modify it with surrounding theme.

picerk50percent_01

When we select images, we get a pretty plain white text on black, when it becomes enabled:

picerk50percent_02

Not the prettiest, but it works pretty well.

The AppBar would have been green from app theme in this case, if I had not wrapped a theme around the picker.

We of course also have the crazy blue dialog as well, that I added just as a demo:

picerk50percent_03

The image delete dialog is behaving OK, since it is using surrounding app themed Text buttons, which is nice, as they then match the app theme, but the exist camera/picker with no images picked, has hard coded style on the buttons, it should really not do that, because it makes it impossible to modify them:

picerk50percent_04

It should be like the image delete version and let package users theme the style they want the dialog to have.

Bu the way, with my latest PR you can now also remove the delete pic confirmation dialog.


Some more theming examples. If I swap the black and white color on the AppBar theme:

                    appBarTheme: const AppBarTheme(
                      backgroundColor: Colors.white,
                      foregroundColor: Colors.black,
                    ),

We get this:

picerk50percent_11

picerk50percent_12

We can now see the hard coded elevation the OutlinedButton is set to use as well. Currently it cannot be changed, it should not really set any elevation and let the surrounding theme define the elevation used on it, if any. Normally OutlinedButton does not use elevation.

Also since an Outlined button, by default uses an outline color that is the same as primary color, the default color on a default themed AppBar in light theme, the outline is not even visible when default theme is used, so it is a bit of an odd button button choice in an AppBar.

Also let's again take a look at an issue with the dialogs. These screens are now from an app using its dark mode theme. We can now see another issue with the exit confirmation dialog, it will display buttons with black dark text on a very dark dialog background, if we just let it use its themed default dialog background color in dark mode, example:

picerk50percent_07

Not so nice, and it cannot be fixed with a surrounding theme. Where as the remove image, works OK, example:

picerk50percent_08

For my next PR suggestion I will address this. It will however be minor breaking with past default style, but it is actually more like a bug fix in my opinion.

I can also make a dialog improvement suggestion that uses platform aware confirm dialog style, so it on iOS gets proper iOS confirmation dialogs, instead of Material one, with option to force Material or Cupertino style too. iOS users tends to get very upset if dialogs and on/off switches (not used in the picker) look "wrong", so we should make it possible to use the right style. Just thinking if I should make platform adaptive dialogs the default or not, it should be the default, but if I make it so then it is a minor style break as well, well might as well just bit the bullet on that. Past always "Material" style would also be available as an option, but it should not really default to that.


Now let us flip the colors back to:

                    appBarTheme: const AppBarTheme(
                      backgroundColor: Colors.black,
                      foregroundColor: Colors.white,
                    ),

and also set the new config options like so:

    // Start with flash OFF and hide flash settings button.
    configs.flashMode = FlashMode.off;
    configs.showFlashMode = false;
    // Only back camera used, and hide flip button.
    configs.cameraLensDirection = 1;
    configs.showLensDirection = false;
    // Do not show an alert when no image was selected when returning.
    configs.showNonSelectedAlert = false;
    // Do not show an alert when removing selected images.
    configs.showRemoveImageAlert = false;
    // Done button style, icon button and hide instead of disable.
    configs.doneButtonStyle = DoneButtonStyle.iconButton;
    configs.doneButtonDisabledBehavior = DoneButtonDisabledBehavior.hidden;

This demos the new IconButton style instead as done button. It is a new feature I added in the not yet landed PR.
With it we can get something like below.

No button if nothing is selected (only close option), flash mode is OFF as before, but it cannot modified since there is no UI for it. There is also no UI showing the disabled swap camera, since we removed it, as it is not needed when only using configuration with back camera (you can still show it as disabled as before). Here is the example:

picerk50percent_09

When you have added some images, the done button now becomes visible, but in this case it is just a plain IconButon that belong better as an AppBar action button. Naturally you can change the IconData for the button to whatever you want, the check button is the default, as it also matches what some of the editing features in the picker uses.

picerk50percent_10

In any case these were just some simple examples of Theme wrapping and some new features, that I hope will be merged soon.


I prefer the simple IconButton in the AppBar an OultinedButton does not really belong there in Material Design guide following apps.

I would also like to have an option to specify the DONE button location, so instead of as an action in the AppBar it could alternatively be in bottom right or bottom left corner, for easier one hand reach access. I'm planning to add and propose that feature as a PR as well.

I am also not a big fan of that the image picker messes with default Material font size in the AppBar. I might look into that as well, but changing it would again be minor past style breaking, but at least it would then match rest of font sizes used in the picker and apps typically.

Changing AppBar font size and style should be based on surrounding theme, when it comes to the AppBar title. For the directory element, the smaller size in the media lib is fine imo. Also title should follow platform built in center logic and not override it for iOS. There is built in logic in the AppBar for when iOS overrides center placement in AppBar automatically. Again, for the media folder it is OK to be start aligned in the media lib, imo.


Oh here is the pending PR with the above mentioned new features #27

Here you can read the proposed change log for it: https://github.com/rydmike/advance_image_picker/blob/flash-expose_ok-icon_appbar-style/CHANGELOG.md

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

No branches or pull requests

3 participants