Skip to content
This repository has been archived by the owner. It is now read-only.

Add option to enable/disable Codemirror #29

Closed
rianrietveld opened this Issue Aug 15, 2017 · 26 comments

Comments

Projects
None yet
6 participants
@rianrietveld
Copy link
Member

rianrietveld commented Aug 15, 2017

The accessibility team has concerns about the implementation of this functionality by default, as we expressed previously in ticket https://core.trac.wordpress.org/ticket/12423.

  • Our main concern is that Codemirror is not accessible for screen reader users.
  • There should be an option to enable/disable Codemirror by the user, to fall back to the current (fully accessible) plain text mode.
  • In the case of the theme and plugin editor and the CSS editor for the customiser, it can be an opt-out, because these aren't essential parts of using WordPress for a regular (non coding) user.
  • If Codemirror is going to be implemented in for example the text editor for posts or the HTML widget, it has to be an opt-in. The text editor there is an essential part of WP's functionality.

On a personal note: I've installed the plugin and it's really good work, as a coder I like it very much.
My personal points of view are at: https://core.trac.wordpress.org/ticket/12423#comment:133

@melchoyce

This comment has been minimized.

Copy link
Collaborator

melchoyce commented Aug 15, 2017

Thanks @rianrietveld, I'll add some opt-out design ideas in here.

Where in the Customizer CSS panel and the code editors would you expect to find this checkbox?

@melchoyce

This comment has been minimized.

Copy link
Collaborator

melchoyce commented Aug 15, 2017

Alternately — would you expect this checkbox to live somewhere else entirely, like user settings?

@SergeyBiryukov

This comment has been minimized.

Copy link
Member

SergeyBiryukov commented Aug 16, 2017

would you expect this checkbox to live somewhere else entirely, like user settings?

Maybe an "Enable accessibility mode" link in Screen Options, like on Widgets screen?

@rianrietveld

This comment has been minimized.

Copy link
Member Author

rianrietveld commented Aug 16, 2017

@SergeyBiryukov yeah, that's an idea for the theme/plugin editor, not sure about the "Enable accessibility mode", maybe just "disable syntax highlighter"?
Although this would not solve the issue for the customizer.

What about adding it to the the user settings (your profile) as it has more checkboxes for customisation like Disable the visual editor when writing and such (or will those settings all be removed in the future?).

In the customiser CSS help text we then could add something like: The syntax highlighter can be disabled with the settings in Your Profile.

I've put this question in the accessibility Slack channel, maybe more people can come up with ideas

@samikeijonen

This comment has been minimized.

Copy link

samikeijonen commented Aug 16, 2017

Screen options is kind of hard to find. I didn't even know there was accessibility mode for widgets couple months ago.

User profile seems better idea next to Disable visual editor like Rian mentioned.

In the customiser CSS help text we then could add something like: The syntax highlighter can be disabled with the settings in Your Profile.

Good idea.

@westonruter

This comment has been minimized.

Copy link
Member

westonruter commented Aug 16, 2017

I know that @afercia wants to get rid of the "accessibility mode" screen option for widgets. I like the setting being on the user profile screen like the opt-out setting exists for the visual editor.

@melchoyce

This comment has been minimized.

Copy link
Collaborator

melchoyce commented Aug 16, 2017

I thought about screen options at first too, but it turns out none of the pages that contain code editors have them.

User setting + notification letting folks know they can disable on each Editor screen seems like a good idea 👍

@westonruter

This comment has been minimized.

Copy link
Member

westonruter commented Aug 16, 2017

I'll open a PR with this.

@westonruter westonruter self-assigned this Aug 16, 2017

@melchoyce

This comment has been minimized.

Copy link
Collaborator

melchoyce commented Aug 16, 2017

Cool. I'm thinking, in user settings:

image

Does that make sense?

@samikeijonen

This comment has been minimized.

Copy link

samikeijonen commented Aug 16, 2017

I'm not sure does it have to say if you're using a screen reader. I rather not point out some group of people.

@melchoyce

This comment has been minimized.

Copy link
Collaborator

melchoyce commented Aug 16, 2017

I'm fine with removing the second sentence if it feels too much like singling out a particular group 👍

We should also remove the period from the end to match the other settings.

@westonruter

This comment has been minimized.

Copy link
Member

westonruter commented Aug 16, 2017

Please review #31.

@cedon

This comment has been minimized.

Copy link

cedon commented Aug 16, 2017

I'm not a fan of having screen reader users be the ones to jump through an extra hoop though. Having opt-out makes them do just that.

I would prefer it be an opt-in on the screen @melchoyce posted. Maybe add a one-time announcement to the editor too, hidden from screen readers(?), that allows them to enable it from there when they use it for the first time.

@westonruter

This comment has been minimized.

Copy link
Member

westonruter commented Aug 16, 2017

If it is opt-in then the vast majority of users who would benefit from the new feature would likely never encounter it, since I'll wager most users aren't visiting their profile frequently to discover new settings to tweak (also given, “Decisions, not options”). Since the majority of users would benefit from having syntax highlighting/checking, I think it should be opt-in by default with screen-reader-text that informs of the user preference to disable the syntax highlighting.

@cedon

This comment has been minimized.

Copy link

cedon commented Aug 16, 2017

@westonruter That's why I also suggested the one-time announcement thing that pops up on initial use. That way it draws attention to the fact this feature exists.

Under normal circumstances, I would be in favor of it being opt-out. But since it's an accessibility issue, I would rather err on the side of assistive technology users having the path of least resistance. It's kind of like a business putting a ramp at the back of the building. A person in a wheelchair can get inside the building but it's an added hurdle for them.

@westonruter

This comment has been minimized.

Copy link
Member

westonruter commented Aug 16, 2017

BTW: the one-time announcements are called “pointers” in WP.

What if there was a one-time pointer announcing syntax highlighting, which then instructs how to opt-out?

Note that the pointer wouldn't help in either case for new installs, as pointers normally only appear upon upgrading.

@cedon

This comment has been minimized.

Copy link

cedon commented Aug 16, 2017

Can it be set to show on first use of the editor as well?

Once the accessibility issues are resolved it should definitely go to opt-out though.

@westonruter

This comment has been minimized.

Copy link
Member

westonruter commented Aug 16, 2017

CodeMirror is not being proposed here for the post editor yet. For that we wait for Gutenberg and CodeMirror accessibility issues being resolved.

@rianrietveld

This comment has been minimized.

Copy link
Member Author

rianrietveld commented Aug 17, 2017

A few things:

  • I'm in favour of an opt out here. It's not vital WP functionality, a lot of coders will love it and I agree with @westonruter that users don't visit their profile often to discover it. But a sentence in on the CSS customiser page and some text on the theme/plugin editors on how to disable it will be useful.
  • I wouldn't focus on screen reader users only, probably also other people would like to disable it too. I agree with @samikeijonen: the text Disable syntax highlighting when editing code is enough
@rianrietveld

This comment has been minimized.

Copy link
Member Author

rianrietveld commented Aug 17, 2017

Thanks @westonruter for the PR and the merge.
Propose for the help text something like:

In the settings of your user profile you can disable the code syntax highlighter and work in plain text mode.

And make disable the code syntax highlighter a link to the settings page.

Location:

  • in the help text of themes and plugins
  • in the help under the question mark with the CSS customizer
@rianrietveld

This comment has been minimized.

Copy link
Member Author

rianrietveld commented Aug 17, 2017

@westonruter
Is it useful to open this again? Because still some help text needs to be added.

@melchoyce melchoyce reopened this Aug 17, 2017

@melchoyce

This comment has been minimized.

Copy link
Collaborator

melchoyce commented Aug 17, 2017

I might switch around the order of your sentence:

You can disable the code syntax highlighter in your user profile. This will allow you to work in plain text mode.

@cedon

This comment has been minimized.

Copy link

cedon commented Aug 17, 2017

If it's just going to be the code editor on the back end, than an opt out would be fine in that case. Here's a best-of-both-worlds thought... could something be added to wp-config.php to flag it as off on a fresh install? Something like define( 'DISALLOW_SYNTAX_HIGHLIGHT', true ); ?

Also, would having define( 'DISALLOW_FILE_EDIT', true ); disable it as well?

@westonruter

This comment has been minimized.

Copy link
Member

westonruter commented Aug 18, 2017

@rianrietveld @melchoyce Please review: #34

@westonruter

This comment has been minimized.

Copy link
Member

westonruter commented Aug 18, 2017

could something be added to wp-config.php to flag it as off on a fresh install? Something like define( 'DISALLOW_SYNTAX_HIGHLIGHT', true );

I don't really see a compelling reason for adding a new constant to configure this. If the feature truly is to be disabled for all users, then this could be implemented via a plugin which could filter get_user_metadata for the syntax_highlighter key and always return 'false', along with hiding the option in the user profile screen via .user-syntax-highlighting-wrap { display:none; }. Or alternatively, there are currently filters for theme_editor_codemirror_opts, plugin_editor_codemirror_opts, and customizer_custom_css_codemirror_opts which you can use to turn off CodeMirror via adding __return_false. Nevertheless, I think these filters may need to be revisited.

Also, would having define( 'DISALLOW_FILE_EDIT', true ); disable it as well?

No, because DISALLOW_FILE_EDIT does not apply to Custom HTML widget or Additional CSS in the Customizer.

@westonruter

This comment has been minimized.

Copy link
Member

westonruter commented Aug 18, 2017

Please see #37 for follow-up questions on accessibility.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.