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

Constrain the size of the ImageCrop for large images. #8

Open
gmerton opened this issue Dec 18, 2024 · 3 comments
Open

Constrain the size of the ImageCrop for large images. #8

gmerton opened this issue Dec 18, 2024 · 3 comments
Assignees

Comments

@gmerton
Copy link

gmerton commented Dec 18, 2024

Feature proposal

Some images have a natural size that is quite large. Users typically expect that image to scale appropriately to fit within the screen. It would be cool if ImageCrop followed that convention. The image below shows an example... note that the cropping tool is barely visible and users have to scroll down to find it.

image

Describe solution expectations

My caveman approach to solving this is to set some styles for the div containing imageCrop. The code below shows the modifications I made to the openCropDialog method of the UploadImageCropDemo class. Note the use of dialogLayout.getStyle().set() method.

As I said, it's quite crude. It would probably be better to set dimensions as percentages and ensure this works on all devices. Nonetheless, it is effective and creates a much better user experience (provided you can ignore many overlapping dialogs, which is my fault not yours) as you can see in the screenshot.

private void openCropDialog(ByteArrayOutputStream outputStream, String mimeType, Avatar avatar) {
        ImageCropGabe imageCrop;
        // Set up image crop dialog
        Dialog dialog = new Dialog();
        dialog.setCloseOnOutsideClick(false);
        dialog.setMaxHeight("100%");
        dialog.setMaxWidth(dialog.getHeight());

        Button cropButton = new Button("Crop image");
        Button dialogCancelButton = new Button("Cancel");
        dialogCancelButton.addThemeVariants(ButtonVariant.LUMO_ERROR);

        String src = getImageAsBase64(outputStream.toByteArray(), mimeType);
        imageCrop = new ImageCropGabe(src);
        imageCrop.setAspect(1.0);
        //imageCrop.setCircularCrop(true);
        imageCrop.setCrop(new CropGabe("%", 25, 25, 50, 50)); // centered crop
        imageCrop.setKeepSelection(true);
        int targetHeight;
        int targetWidth = 175;
        int naturalWidth = 0;
        int naturalHeight = 0;
        try {
            // Decode base64 back to a byte array
            byte[] decodedBytes = Base64.getDecoder().decode(src.split(",")[1]); // Removing "data:image/...;base64,"
            ByteArrayInputStream inputStream = new ByteArrayInputStream(decodedBytes);

            // Use ImageIO to read the image dimensions
            BufferedImage bufferedImage = ImageIO.read(inputStream);
            if (bufferedImage != null) {
                naturalWidth = bufferedImage.getWidth();
                naturalHeight = bufferedImage.getHeight();
            }
            logger.info("Image dimensions: " + naturalWidth + " x " + naturalHeight);
        } catch (Exception e) {
            logger.error("Failed to load image dimensions: " + e.getMessage(), e);
        }

        double aspectRatio = naturalWidth > 0 && naturalHeight > 0 ? (double) naturalHeight / naturalWidth : 1.0;
        targetHeight = (int) (targetWidth * aspectRatio);

        logger.info("target dimensions: " + targetWidth + ", " + targetHeight);
        cropButton.addClickListener(event -> {
            byte[] newCroppedPicture = null;
            newCroppedPicture = imageCrop.getCroppedImageBase64();
            avatar.setImage(imageCrop.getCroppedImageDataUri());
            dialog.close();
        });
        dialogCancelButton.addClickListener(c -> dialog.close());

        HorizontalLayout buttonLayout = new HorizontalLayout(dialogCancelButton, cropButton);
        Div dialogLayout = new Div(imageCrop);
        dialogLayout.setId("imageCropContainer");
        dialogLayout.getStyle()
                .set("width", targetWidth + "px")     // Scale the container
                .set("height", targetHeight + "px")  // Maintain desired height
                .set("overflow", "hidden")           // Crop overflow content
                .set("margin", "0 auto");
        buttonLayout.setWidthFull();
        buttonLayout.setJustifyContentMode(FlexComponent.JustifyContentMode.END);
        dialog.add(dialogLayout);
        dialog.getFooter().add(buttonLayout);
        dialog.open();
    }

Screenshot 2024-12-17 at 8 38 34 PM

Additional information

No response

@github-project-automation github-project-automation bot moved this to Inbox (needs triage) in Flowing Code Addons Dec 18, 2024
@paodb
Copy link
Member

paodb commented Dec 18, 2024

This was already reported on issue #5. If you use imageCrop.setImageFullHeight(true); then the image will occupy the full viewport height. See details here.

@javier-godoy javier-godoy added the question Further information is requested label Dec 18, 2024
@gmerton
Copy link
Author

gmerton commented Dec 18, 2024

@paodb

Oh cool, I wasn't aware of that, thanks for pointing it out. I tried it and it's a big improvement!
I still think there's a little room for refinement though. In the "Upload" use case, the imageCrop will scale down to the viewport height as expected, but the dialog itself is shorter than the viewport. That means for large images, there will still be an unwanted scrollbar on the dialog (see screenshot).

Would it make sense to set the imageCrop height relative to its container rather than the viewport? In 99% of cases, the containing component will be smaller than the viewport so the viewport feels like a slightly unnatural basis for scaling child components. Just my 2 cents... thanks for all your work!

Also, what are your thoughts on setting imageFullHeight=true by default?

Screenshot 2024-12-18 at 9 03 27 AM

@javier-godoy
Copy link
Member

Hello. Thanks for calling this into our attention. A height of 100% does not work (it does not scale the image to the size of the dialog), while a height of 100vh is a compromise solution: it scales the image, but it's still a little big taller than the dialog, but it's worth exploring an alterative approach.

Also, what are your thoughts on setting imageFullHeight=true by default?

I haven't yet analyzed how it would affect the different use cases (large image vs. small image, constrained vs. unconstrained container size, inside a dialog, etc.). Let's discuss it in #12. It would be a breaking change, but I'm not against breaking changes when they improve the developer experience.

@paodb paodb removed the question Further information is requested label Jan 16, 2025
@paodb paodb moved this from Inbox (needs triage) to To Do in Flowing Code Addons Jan 16, 2025
@paodb paodb self-assigned this Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: To Do
Development

No branches or pull requests

3 participants