narendrasinghrathore/sketch-assistant-spell-check

Feedback needed

narendrasinghrathore opened this issue · 4 comments

narendrasinghrathore commented

Hi @christianklotz @jedrichards @michelegera
I have developed a sketch assistant that let you check spelling mistakes in your sketch files and list all of them.
Also, try to give a suggestion(s).

I hope you can run the plugin and let me know what you think.
https://drive.google.com/file/d/1XtejYL8uvEK_l9XMYDHOACYqaO9i5ulz/view?usp=sharing

Waiting for your feedback.

Regards
Narendra Singh

Note:
I have tested on my Windows OS machine by running a unit test ( as sketch doesn't support Windows OS).

christianklotz commented

Great work on the spell checker! Glad to see the test driven approach worked for you on Windows 😊 Your Assistant is working nicely, as expected within Sketch as you can see below 🎉

image

A few thoughts:

  1. I had to replace rmdir /s/q dist in the build script in package.json with rm -rf, probably because you're using Windows and I'm on macOS. Maybe there's a command that works cross platform.
  2. Make sure to use your Assistant name, sketch-assistant-spell-check or perhaps better sketch-spellcheck-assistant, sketch-simple-spellchecker-assistant or something similar across your code base. For the last two you could perhaps import the package.json using import pkg from '../package.json and use pkg.name?
  3. Consider changing the rule title to "Use correct spelling", i.e. make it clear what is expected from the user, see https://developer.sketch.com/assistants/rules-and-reports for best practice.
  4. Similar for the rule description, maybe better "Use correct spelling in your text layers" to instruct the user what to do.
  5. Instead of textNoLoremIpsum use a variable name that defines more what the rule does, e.g. correctSpelling or spellCheck
  6. I don't think @skpm/fs should be a dependency. It can only be used within the Sketch Plugin JavaScript environment which dynamically exposes the Objective-C runtime and is entirely different from the Assistants environment.
  • 👍1
  • 🚀1
jedrichards commented

Nice work! 👏 😄

Bear in mind that Assistant rules are run every time the document changes, and the code inside your text layer loop runs for every text layer in the document. So you want to do the minimal amount of work possible here, since they are hot code paths.

For example I note you're transforming your entire word list to lowercase every time you search it. If possible you want to move as much work as possible outside of your rule function, and make word lookups as fast as possible (perhaps different data structures could be explored, rather than a plain array, I'm not sure).

sketch-assistant-spell-check/src/index.ts

Line 71 in 3b6d091

const misspelled = list.find(w => w.toLowerCase() === transformedWord) !== undefined ? false : true;

  • 👍1
narendrasinghrathore commented

@christianklotz @jedrichards Thanks for taking the time and giving valuable feedback.
Will improve definitely.

Stay tuned for updates ✨✨🎁🎁

narendrasinghrathore commented

@christianklotz

  • I had to replace rmdir /s/q dist in the build script in package.json with rm -rf, probably because you're using Windows and I'm on macOS. Maybe there's a command that works cross platform.
"test": "jest --no-cache",
"build": "rm -rf /s/q dist && npm run build:node && npm run build:sketch",
"build-windows": "rmdir /s/q dist && npm run build:node && npm run build:sketch", // For windows
"build:node": "tsc",
"build:sketch": "webpack",
"package-tarball": "npm run build && npm pack",
"ptw":"npm run build-windows && npm pack" // For windows

@jedrichards

  • Text transformation while looping removed and also limiting the iteration for suggestion list. Improvement of 2 sec for same test files.

Please test and let me know your valuable feedback.

sketch-spellcheck-assistant-1.0.1.zip

  • 👍2