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

Markdown Suggestions #37

Merged
merged 14 commits into from
Jul 18, 2022
Merged

Markdown Suggestions #37

merged 14 commits into from
Jul 18, 2022

Conversation

kendallgassner
Copy link
Collaborator

@kendallgassner kendallgassner commented Jul 11, 2022

Markdown Accessibility dev-tools

I am working to create a markdown dev-tools validator. The validator will check any textArea that:

  • is not hidden on the screen i.e. clientWidth=0
  • has content i.e. value != ''

Working Rules

  • checks that written markdown has a heading
  • ensures that written markdown does not use an h1 tag
  • ensures that written markdown includes descriptive links

Features:

  • On non-github pages the validation Button and the reset button are disabled.
  • The reset button will clear all of the error messages, and remove any added focus rings.
  • The Find TextArea button will highlight the markdown field that the error stems from

A snippet of what the markdown dev-tools looks like:

Markdown accessibility dev-tools showing results of a markdown validation. The results show errors in one textareas

Questions:

  • not sure If I we should be using the Mona illustration

Future TODOS:

  • I would love to use primer components and replace a lot of the styling in panel.css
  • We should internationalization the non-descriptive link text array so users who do write their markdown in english can also use this feature.
  • validate image alt text
  • Report bugs individually. For example we report the "Non Descriptive Links" error once ... even if multiple links are not descriptive but it would be nice to report each link individually to provide more descriptive feedback.

@kendallgassner kendallgassner changed the title [WIP] [WIP] Markdown Suggestions Jul 11, 2022
@@ -0,0 +1,3 @@
module.exports = {
presets: [["@babel/preset-env", { targets: { node: "current" } }]],
};
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added for jest testing

@kendallgassner kendallgassner changed the title [WIP] Markdown Suggestions Markdown Suggestions Jul 14, 2022
@kendallgassner kendallgassner marked this pull request as ready for review July 14, 2022 21:41
@@ -0,0 +1,2 @@
// Create a tab in the devtools area
chrome.devtools.panels.create("GitHub Markdown a11y", "./devtools-feedback/toast.png", "./devtools-feedback/panel.html");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we use "Mona's Markdown a11y" everywhere instead of "GitHub Markdown a11y"?

@inkblotty
Copy link
Collaborator

🎉 👏 🥳

@khiga8
Copy link
Owner

khiga8 commented Jul 14, 2022

This is so exciting! Thank you for opening this PR (and also adding tests) 👏!

Some initial thoughts/questions!

  1. Is there a reason you decided to provide a validation mechanism through the dev panel versus providing immediate feedback directly on the page? (e.g. appending textual feedback onto the Preview editor). This is non-blocking by the way, just curious!
  2. Regarding the rule: checks that written markdown has a heading. I don't believe this rule applies to markdown comments but I got flagged. Is this rule appropriate?
  3. When I switch to the Preview tab without submitting and run the Validate Markdown, it doesn't seem to validate the markdown. Is that expected?
  4. The links on the panel appear the same as the surrounding static text. I think we should add an underline or some other styling.

@kendallgassner
Copy link
Collaborator Author

When I switch to the Preview tab without submitting and run the Validate Markdown, it doesn't seem to validate the markdown. Is that expected?

Right now I am querying by textArea 🤔 -- because these pages have several hidden textAreas I have had to filter by hidden textAreas... I believe the when a user selects preview the textArea becomes hidden and therefore the textArea is removed from the validation list :/ .... We might be able to optimize this by adding an data attribute of some sort to edited textAreas in the future

@kendallgassner
Copy link
Collaborator Author

The links on the panel appear the same as the surrounding static text. I think we should add an underline or some other styling.

Thank you for catching this! I had these links styled so something might have gotten messed up let me fix 👩‍🎨

@kendallgassner
Copy link
Collaborator Author

Is there a reason you decided to provide a validation mechanism through the dev panel versus providing immediate feedback directly on the page? (e.g. appending textual feedback onto the Preview editor). This is non-blocking by the way, just curious!

I think to avoid noise as the user is typing -- I don't want to provide feedback until the user is ready

@kendallgassner
Copy link
Collaborator Author

kendallgassner commented Jul 15, 2022

regarding the rule: checks that written markdown has a heading. I don't believe this rule applies to markdown comments but I got flagged. Is this rule appropriate?

Disabling the missing heading rule for now because sometimes title fields are separate from the main textarea:

an issue form where the title of the issue has a separate input field then the overall text area

Copy link
Owner

@khiga8 khiga8 left a comment

Choose a reason for hiding this comment

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

Great work!!! Let's ship it!!!! 💪🚀

@khiga8
Copy link
Owner

khiga8 commented Jul 18, 2022

I would also recommend creating follow-up issues to document the future to-dos you listed!

@kendallgassner kendallgassner merged commit 01b2db9 into main Jul 18, 2022
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