Skip to content

changes to card rendering through json and description box modification#3

Open
krishnak011 wants to merge 16 commits into
gramener:mainfrom
krishnak011:network-enhancement
Open

changes to card rendering through json and description box modification#3
krishnak011 wants to merge 16 commits into
gramener:mainfrom
krishnak011:network-enhancement

Conversation

@krishnak011

Copy link
Copy Markdown

No description provided.

@sanand0

sanand0 commented Oct 25, 2024

Copy link
Copy Markdown
Contributor
  • Use marked via cdn.jsdelivr.com
  • In #description-box remove all card-related stuff. Avoid text-center -- let it left-align. Simplify the HTML
  • Avoid styles for #description-box -- use Bootstrap classes for margin & padding instead
  • Send clean code for the pull request. No console.log()
  • Ask an LLM to review the PR to see if you're aligning to the coding conventions of the rest of the code. (Please send me the resport)
  • Use camelCase (not $DescriptionBox)
  • Prefer const to let
  • Shorten and simplify the code. (Or ask an LLM to do it.) But don't compromise readability. Readability is not about comments. It's about naming variables and keeping flow simple.
  • Prefer await to callbacks (e.g. for fetch)
  • Rather than adding event listeners to the lit-html generated events, use the lit-html event helper, OR bind the parent DOM element generated by your HTML and delegate the event
  • See if you can use .classList.toggle() instead of checking if it exists and adding/removing
  • Avoid setting value= for the default option, i.e. avoid <option selected value="Count">. Leave it as value=""
  • Do not include any code not related to this PR. Keep it completely independent, e.g. URL updates
  • Format with Prettier using a print width of 120
  • Check off items on this list as you progress

@krishnak011

Copy link
Copy Markdown
Author

All the changes made, removed the marked for now since it is not used in current PR.

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.

2 participants