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

Added contextual image to AltImage #694

Closed
wants to merge 3 commits into from

Conversation

parion
Copy link

@parion parion commented May 17, 2023

Adds a feature that allows users to see the image they are adding alt text to as described in #658

By default the image in the Alt Text modal will be shown fully and then it's height will be collapsed so the user can partially see the image, text box, and keyboard, along with the CTAs.

image image

@pfrazee
Copy link
Collaborator

pfrazee commented May 17, 2023

I really appreciate the thought you put into this, but I think there's a simpler option!

Rather than collapsing the image, what if we kept it full-sized but then put everything in a scrollview and autofocused the input so that the UI takes us down to it straight away? Then you can easily scroll up to reference your image

@pfrazee pfrazee added the x:changes-requested The contribution needs updates before merging label May 17, 2023
@parion
Copy link
Author

parion commented May 17, 2023

Great advice @pfrazee, it looks a whole lot better now! The entire modal is now contained within a ScrollView and a automaticallyAdjustKeyboardInsets property offsets the keyboard height on iOS. I am not able to test on Android, but I believe the issue wasn't present there as the entire app is offsetted by the keyboard. The TextInput is also focused on modal open as it's likely the user is interested in entering Alt text when they click Alt.

I've pushed this code to this branch and this is what it looks like:
image

Something else I played with is this. The image is still contained within a ScrollView, but the bottom elements are not, letting them remain "stickied" to the bottom. By using a KeyboardAvoidingView, the elements are pushed up and remain in view while the picture is also in view (missing the black background, I'm lazy). I think this solution would be best if we're worried about images being too big. Just wanted to know your thoughts on this.

image

@TerminalFi
Copy link

For the second, it might look better if the background behind the submit was semi-opaque. Right now it looks horrible being over the image

@parion
Copy link
Author

parion commented May 18, 2023

For the second, it might look better if the background behind the submit was semi-opaque. Right now it looks horrible being over the image

Absolutely agree. As I mentioned I was lazy and it was more of showing off a quick idea I had and not bothering to properly style it.

@pfrazee
Copy link
Collaborator

pfrazee commented May 19, 2023

Sweet, thanks. When I get some time I'll take this to the finish line

@pfrazee pfrazee added x:planned We're on it! and removed x:changes-requested The contribution needs updates before merging labels May 19, 2023
@pfrazee
Copy link
Collaborator

pfrazee commented Jun 27, 2023

Thanks again everybody. I ended up tackling this over here #910

Cheers

@pfrazee pfrazee closed this Jun 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
x:planned We're on it!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants