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

add documentation for remove emoji #216

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jazzsequence
Copy link
Contributor

@jazzsequence jazzsequence commented May 8, 2020

  • adds documentation for the remove-emoji module

See #96

Copy link

@hm-linter hm-linter bot left a comment

Choose a reason for hiding this comment

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

Linting failed (4 errors).

4 notices occurred in your codebase, but none on files/lines included in this PR.

docs/remove-emoji.md Outdated Show resolved Hide resolved
@roborourke roborourke added this to the 5.0 milestone Jun 16, 2020
@roborourke
Copy link
Contributor

I'm kind of in 2 minds here. In the original issue I think I chose a bad name for the config option. Talking about removing emoji is a bit confusing and drastic sounding as we're really just removing the image replacement functionality. We'd need to be very clear about that in the wording throughout.

Do you think the general explanation of what emoji are and the details about the specific JS files that are no longer loaded might be more than is really needed? I think it just needs to explain that if your website might be run on systems that do not support emoji you can enable image replacements by setting this setting.

In a similar pattern to the search and SEO module docs maybe we could condense this into a small section on the main readme if there isn't enough to warrant a full page.

I might make an issue for renaming that config option first (we can keep back compat) but I think it'll help with making these docs clearer.

@jazzsequence
Copy link
Contributor Author

@roborourke I think renaming the config option is a better first step. As you say, "remove emoji" sounds pretty drastic and misleading, and the description, I think, makes less sense if you go into it with that frame of reference. As such, we should punt this to 6.0 and make changing the option name a dependency of this.

@jazzsequence jazzsequence modified the milestones: 5.0, 6.0 Oct 8, 2020
@@ -0,0 +1,27 @@
# Remove Emoji Image Replacement

Remove all the emoji scripts that are included with WordPress. By default, this module is active. To allow emojis, toggle the `remove-emoji` module in the Altis config.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

"To allow emojis" should be "To allow emoji image replacement"

@roborourke
Copy link
Contributor

Created #260 for the setting name change

@jennybeaumont jennybeaumont removed this from the 6.0 milestone Feb 5, 2021
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.

3 participants