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

Fix empty image fields saving null or undefined. #829

Merged
merged 1 commit into from
Nov 27, 2017
Merged

Conversation

tech4him1
Copy link
Contributor

- Summary

Closes #828.

Does anyone think we need a more through fix, i.e. returning an empty string instead of null or undefined?

- Test plan

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@verythorough
Copy link
Contributor

I think an empty string is great.

I suppose we could think about popping up a confirmation about being sure you actually want screen readers to completely skip this image, but that would get really annoying if you're using a lot of decorative images on a page. I think having the alt field is enough to nudge people in the right direction, and we don't need to teach them accessibility good practices beyond that. :)

@tech4him1
Copy link
Contributor Author

I was wondering if we should send an empty string to all plugins, instead of undefined (vs just changing this for the image plugin). That could be a breaking change, though, so I'm not too keen on it.

Copy link
Contributor

@verythorough verythorough left a comment

Choose a reason for hiding this comment

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

Nice catch on the spelling error!

I wonder if we need the default empty string for the src. Kinda seems like if that field should be required for the block, or if it's empty, the whole tag goes away.
Failing that, we might consider using "no image selected" as the default for the src, instead of an empty string.

@tech4him1 tech4him1 changed the title WIP: Fix empty image fields saving null or undefined. Fix empty image fields saving null or undefined. Nov 18, 2017
@erquhart erquhart merged commit 5271e0f into master Nov 27, 2017
@erquhart
Copy link
Contributor

@verythorough I agree this could use more thought moving forward. As it stands it's an improvement, so merging for now.

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.

Allow empty alt attribute in markdown editor images
3 participants