removes between 1-6 layers of uneeded div nesting #240
Conversation
| </div> | ||
| <EditableH1 | ||
| text={cell.text} | ||
| id={`cell-title-${cell.id}`} |
There was a problem hiding this comment.
cell-${cell.id} is used to target this element for scrollTo purposes, so I believe this change will break that behavior?
There was a problem hiding this comment.
prior to this change there was multiple cell-${cell.id} instances which was doing weird things
There was a problem hiding this comment.
the ids are unique so this shouldn't be problematic. What were the weird things that were happening?
| @@ -29,7 +29,7 @@ | |||
| </script> | |||
| </head> | |||
| <body> | |||
| <div id="root"></div> | |||
| <div id="root" class="flex flex-col"></div> | |||
There was a problem hiding this comment.
Agree with most of these changes, but I prefer we have 0 styling in this file. It makes for a cleaner root and need to open less files when "looking for the rule".
I would add this back to the Layout
| )} | ||
| </div> | ||
| <div | ||
| className={cn( |
There was a problem hiding this comment.
I haven't tried to refactor this yet but I suspect we can improve states here as well.
| basicSetup={{ lineNumbers: false, foldGutter: false }} | ||
| /> | ||
| </div> | ||
| <CodeMirror |
There was a problem hiding this comment.
I'm 50/50 on the readability improvement on this one. A surrounding div with only styles is easier to see than one of many props. Don't feel super strongly.
| @@ -240,7 +238,7 @@ function Error(props: { error: string | null }) { | |||
| <div className="px-1.5 pb-1.5"> | |||
| <div className="bg-error text-error-foreground text-sm font-medium py-2 pl-[10px] pr-3 flex items-start gap-1.5 rounded-sm"> | |||
| <div className="shrink-0 mt-0.5"> | |||
| <Info size={16} /> | |||
| <Info size={16} className="shrink-0 mt-0.5" /> | |||
There was a problem hiding this comment.
Did we improve anything here? We should either remove the div around it or not do this?
Make sure to test this one, I'm not sure it's equivalent behavior (all for it if it is)
|
@versecafe should we close this for now given the large set of view changes in #267 ? |
Just a cleanup PR pulling out several dozen instances of uneeded excessive div nesting for styling by directly applying styles to components, makes dom faster to parse, bit easier for screen readers, and just easier to navigate through while debugging.