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

for - dev: planning and all features #26

Open
wants to merge 75 commits into
base: main
Choose a base branch
from
Open

for - dev: planning and all features #26

wants to merge 75 commits into from

Conversation

AlinaTaoRao
Copy link
Collaborator

@AlinaTaoRao AlinaTaoRao commented Mar 4, 2022


name: for dev branch: including planning and all feature branches
about: A template PR for contributing to this project

close #13
close #25

Checklists

  • All CI checks pass
  • The branch works when you pull it and run it locally

HTML

  • the code is well-formatted
  • the HTML code passes validator.w3.org
  • there are no inline styles (example: style='color: red;')
  • there are no <style> tags with CSS, all styles are hrefs
  • there is no inline JavaScript (example: onclick='doSomething()')
  • there are no <script> tags with JS, all JS is in an separate file
  • ids are used for JavaScript only, not for CSS
  • semantic tags are used
  • ARIA labels are used where necessary
  • spelling and grammar is correct in all site content

CSS

  • the code is well-formatted
  • the code does not use any # id's
  • styles are responsive

JavaScript

  • code is clean and easy to read
  • helpful variable names are used
  • all comments are clear and help to understand the code
  • there is no unused code in comments
  • all file names are helpful and match their exports

src/data.js

  • the file does not import anything
  • the file only exports JS data (primitives, arrays, objects, ...)
  • the file does not contain any functions or logic
  • (optional) the data is documented with JSDocs

src/init

  • there is an index.js that is included by the index.html file
  • ../listener files are imported
  • there is no code that needs to run after the page is initialized
  • any other files have helpful names

src/listeners

  • the DOM can be queried to find elements
  • the DOM is not be modified
  • handlers are imported and used as callbacks to event listeners
  • no other functions are imported or used
  • the file name makes sense for the listener

src/handlers

  • handlers have a JSDoc comment
  • the function name matches the file name
  • handlers are used as a callback to .addEventListener somewhere in the program
  • functions from /logic or /procedures files may be imported and called
  • functions from /handlers may be imported and attached to the DOM with event listeners
  • data from /data.js may be imported and used
  • handlers may read and write to the DOM
  • handlers do not return values that you will need later in the program

src/components

  • A DOM element is returned
  • handlers can be imported and used as callbacks to event listeners
  • logic, components and procedures can also be imported
  • there is an HTML test file to render the component with different inputs
  • each component has a .test.html file
  • bonus points for a unit test .spec.js file

src/logic

  • the handler has a complete and correct JSDoc comment
  • the function name matches the file name
  • other functions from /logic may be imported and called
  • the function does not read or write to the DOM
  • the function does not log to the console
  • each logic function has a .spec.js file
    • tests are simple with no extra code
    • tests are well-named
    • tests cover common use cases (bonus for edge cases)
    • tests check for side-effects (if necessary)

@AlinaTaoRao AlinaTaoRao linked an issue Mar 8, 2022 that may be closed by this pull request
74 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

for -dev: dev branch dev strategy version 2
1 participant