MetaFam / TheGame

The platform that MetaGame will be played on aka MetaOS - an open source framework for running decentralized societies. Currently featuring MyMeta Profiles, Dashboard & Quests
https://metagame.wtf
Other
131 stars 78 forks source link

Frontend Guide: Code Conventions #249

Open TheLoneRonin opened 3 years ago

TheLoneRonin commented 3 years ago

Problem

Currently, the frontend guide helps you get started and understand how to write urql queries. However, it does not outline conventions and best practices for implementing actual React code.

Improving the frontend guide to include these outlines will help ensure that going from prototyping to an actual merge will be as easy as possible and also cost reviewers less time to approve the diffs.

Solution

pacoccino commented 3 years ago

Not meant to go in the guide, but some advices from the drawer PR:

TheLoneRonin commented 3 years ago

@pakokrew, agree that a lot of the app drawer issues were visual design related and that's not as related. However, there are still things we could write up that helps reduce code review friction.

I'm definitely going to send in a PR for us to review of general code conventions we should be following.

dysbulic commented 3 years ago

I personally think we ought to not use semicolons. I'm almost always on the side of the solution that requires less typing & looks cleaner.

peth-yursick commented 1 year ago

Improving the frontend guide to include these outlines will help ensure that going from prototyping to an actual merge will be as easy as possible and also cost reviewers less time to approve the diffs.

Any idea if this was ever done @dysbulic @alalonde?

alalonde commented 1 year ago

nope

peth-yursick commented 1 year ago

nope

should it be?

alalonde commented 1 year ago

If we start seeing an influx of new builders, yes, otherwise meh

On Oct 15, 2022, at 1:45 PM, peth-yursick @.***> wrote:

nope

should it be?

— Reply to this email directly, view it on GitHub https://github.com/MetaFam/TheGame/issues/249#issuecomment-1279820325, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKWNVWFGXTEBYVVBF2E4ZDWDMCVLANCNFSM4VEKL2AQ. You are receiving this because you were mentioned.

dysbulic commented 1 year ago

A bunch of assets for the recently developed pages are obviously rasterized vector images and that irks me.

Part of this @peth-yursick is your sources. What software did you use to design your flowcharts? It'd be great if they were SVGs, both for SEO purposes & if the developer needs to tweak positioning or text to work with the layout.

There are some common conventions I find myself creating suggestions for on a regular basis. I could mention some of those. Stuff like destructuring where possible and not using conditionals to return booleans when you can just return the value of the conditional — anti-patterns.

I'll contemplate & jot something down here once I'm a bit more on top of the ESM upgrade.

luxumbra commented 1 year ago

I'd like to see a section to remind developers who work on the frontend to make sure they test their work on a range of device sizes or at the bare minimum on mobile. According to Google Analytics we get around 34% users on mobile and there's quite a few places where this has clearly not been done. This is particularly noticeable with the new Quest Chains integration - it is really not an enjoyable UX on mobile.

dysbulic commented 1 year ago

Prefer attribute={{ var1: val1, var2: val2 }} when the overall length is less than 60 characters.

dysbulic commented 1 year ago

Don't use semicolons. I want to switch Prettier to remove them & then ditch Prettier.

Another rule would be indentation is always accompanied by opening or closing grouping symbols. A line before an indent must end in an opening grouping symbol. A line which is deindented must begin with a closing symbol and match the symbols opened on that line.

So:

const var = (
  generator
  .then((result) => process(result))
  .then((next) => doMoreWith(next))
)

and

const var = (
  aLongInstruction()
)

Never:

const func = ({ args, in, a, list }) =>
  doSomethingWith(args, list)
continuationOfTheCode()

Prettier doesn't respect this rule which is why it needs to be turned off. If you always have close symbols it creates diagonal lines when closing several statements simultaneously and can make balancing grouping symbols easier.

dysbulic commented 1 year ago

Put spaces between the braces that delineate an object and the contents of the object.

Put spaces after the comma separating pairs in an object.

Do not put a space between the brackets and the array contents they surround.

dysbulic commented 1 year ago

All the letters of a normally all caps word are capitalized in the camel case spelling (unless it is the first word, then they're all lowercase):

alalonde commented 1 year ago

I am strongly in favor of keeping Prettier. It saves me so much time and is simple tooling for maintaining consistent styling.

dysbulic commented 11 months ago

I'd like to keep this around. I am working on a mob programming interface where members of the mob swap in and out, but there's a continual presence of at least three people.

One of the big criteria for evaluating the Driver (only one person operates the keyboard at a time) is their ability to format the code they're asked to write according to the code conventions, thus I'd like to gather a few more of those.