search

glam-lab / degender-the-web

A Chrome extension that replaces all gendered pronouns with "they/them/their."
GNU Affero General Public License v3.0
6 stars 3 forks source link

Use "gender" as a stopword only when it's visible #90

Closed beszel closed 5 years ago

beszel commented 5 years ago

Resolves #49. @ProfJanetDavis If I should be handling the commit history differently – for example, if I should be squashing commits or something – please let me know! Another area that may need attention is whether or not the tests have been implemented in the correct files.

ProfJanetDavis commented 5 years ago

Also - yes, please do squash these commits! I'd like to see the tests and new code together in one commit. Ideally, you'd have written the tests before the code (see https://www.agilealliance.org/glossary/tdd/), though of course that wasn't possible here as I was still building out the test architecture.

beszel commented 5 years ago

What do you think of including the link and hidden elements in helper.html? This ticket says that an invisible instance of the word "gender" should do nothing, so we might want to modify the page being tested rather than add new tests that mirror existing tests on a new page. Does that make sense? One drawback of this option is that the messages shown for test failure won't say anything about hidden elements. That could make debugging more difficult.

ProfJanetDavis commented 5 years ago

I like the idea of not proliferating HTML pages for testing. Yes, go ahead and do that.

beszel commented 5 years ago

I did a force-push because I mistakenly reset some changes. I hope to avoid that in the future by being more mindful of the remote. This last commit was easier than writing new end-to-end tests, and it should either address or make irrelevant each of the comments above. Let me know if this needs anything else!