feat(cdr): Approach 3 - Initial UI (port over cdr/m components)#8
Conversation
| @@ -0,0 +1,125 @@ | |||
| import Button, { ButtonProps } from "@material-ui/core/Button" | |||
There was a problem hiding this comment.
This file was brought in unchanged
| @@ -0,0 +1,30 @@ | |||
| <!DOCTYPE html> | |||
There was a problem hiding this comment.
This HTML file is a placeholder. Likely we'll have to do something like v1 where the backend injects some metadata into the HTML when serving it to the client.
There was a problem hiding this comment.
| @@ -0,0 +1,216 @@ | |||
| import { useEffect, useRef } from "react" | |||
There was a problem hiding this comment.
This was brought in mostly unmodified (just a quick tweak to localize it to a specific element)
| @@ -0,0 +1,74 @@ | |||
| import React from "react" | |||
There was a problem hiding this comment.
This was brought in mostly as-is from v1, except I removed some properties that weren't yet needed.
| @@ -0,0 +1,20 @@ | |||
| import * as React from "react" | |||
There was a problem hiding this comment.
Brought in as-is from v1
…nto bryphe/prototype/3-port-ui
bryphe-coder
changed the title
Codecov Report
@@ Coverage Diff @@
## main #8 +/- ##
==========================================
+ Coverage 69.46% 70.44% +0.97%
==========================================
Files 18 18
Lines 1130 1130
==========================================
+ Hits 785 796 +11
+ Misses 273 264 -9
+ Partials 72 70 -2
Continue to review full report at Codecov.
|
|
FYI @kylecarbs - I made the changes we discussed earlier today:
|
kylecarbs
left a comment
kylecarbs
left a comment
There was a problem hiding this comment.
We could cleanup the theme files a bit. I know they're copied from the other repo, but I think some minor love could set better precedent on this repo.
Chefs choice on all the things I commented. Overall looks great to me!
| # Check that node is available | ||
| # TODO: Implement actual test run | ||
| - run: node --version | ||
| - run: yarn install |
There was a problem hiding this comment.
We should probably cache node modules here too!
There was a problem hiding this comment.
For sure! I'll add this in a separate change
| } | ||
| } | ||
|
|
||
| // END Material-UI v5 code: |
There was a problem hiding this comment.
I know we're just pasting these, but I think this comment could be removed.
There was a problem hiding this comment.
This is actually material-ui v5 code (we're on material ui v4).
One thing we could consider is upgrading to material-ui v5 for this... but it means the components might not be as easy port from v1 -> v2. There are several breaking changes, including import paths: https://mui.com/guides/migration-v4/
My thinking is we should stick with what we have for the moment to make it easy to port v1 components in -> especially given that in 'v2 Phase 2' it's possible we'd redesign the UX and/or move away from material-ui.
| @@ -0,0 +1,118 @@ | |||
| import { Palette } from "@material-ui/core/styles/createPalette" | |||
| /** | |||
There was a problem hiding this comment.
We use multiple // above, and a block here. It would be nice to standardize and enforce a style with linting.
There was a problem hiding this comment.
I'll take a look at this in a subsequent PR.
We could enable this rule: https://eslint.org/docs/rules/multiline-comment-style (ie, starred-block for multiline comments)
This refactoring re-organizes the `site` folder to have a nested `src` folder. Originally, [we wanted to keep the directory structure shallow](#8 (comment)) - but there were two points that motivated this change to introduce the `src` level. 1. We have several non-`src` folders now (`e2e`, `static`, `html_templates`, `.storybook`) 2. Having a `src` folder makes it easier to run XState Typegen So given those two data points - I believe it makes sense to revisit that and introduce a `src` folder.
buildReplacementLines' inserted branch inherited cLead from an
arbitrarily-picked paired content line, so a 4sp LLM search
inserting into a tab-indented or 2sp-indented file emitted the
wrong indent depth. The bug fires on every wrap-in-block,
single-to-multi expansion, or insert-new-block edit where the
caller's search/replace whitespace unit differs from the file's.
Scoped fix:
- detectIndentUnit(lines) scans leading whitespace and returns the
smallest consistent unit (one tab, or N spaces via GCD). Mixed
tab+space samples return ("", false).
- translateIndentLevel(rLead, sLead, cLead, searchUnit, fileUnit)
computes the caller's rep_level relative to their search_base and
emits the same relative level in the file's unit.
- buildReplacementLines computes searchUnit (from search+rep) and
fileUnit (from matched) once per splice. Inserted lines (rMid
non-empty, no search pair) call translateIndentLevel and fall
back to the previous cLead-inheritance path when detection fails
or a lead is not a clean multiple of its unit.
Pinned contracts (escape hatch, caller-agrees-with-itself,
empty-body carve-out, ending normalization) all stay green: only
the inserted branch changes, and paired lines take neither of the
new helpers.
Flips the 5 reds in TestEditFiles_FuzzyIndent_InsertionLevelAware
from red to green. Lock #7 and #8 update their expected outputs
to reflect partial post-fix correctness (inserted-line bugs
fixed, middle-substitution leakage still present for follow-up).
Lock #6 (Unwrap) and TestEditFiles_ReplaceAll_FuzzyIndentGap are
middle-substitution cases unchanged by this fix.
Addresses DEREM-11 (diff-aligned pairing known-limitation) for
the pure-insertion class. Middle-substitution level-awareness
remains a separate follow-up tracked by the middle-rewrite locks.
2 participants
This is testing out Approach 3 in the UI scaffolding RFC.
Fixes #11
The folder structure looks like:
sitecomponents(buttons, empty state, etc)pages(large sections of UI -> composition of components)theme(files defining our palette)Several components were able to be brought in essentially unmodified:
SplitButtonEmptyStateFooterOther components had more coupling, and need more refactoring:
NavBarConfettiCurrent State:
For a full working app, there's potentially a lot more to bring in:
(and all the supporting dependencies)
TODO:
<EmptyState />to component<NavBar />, streamline for V2<Footer />, streamline for V2page/Workspaces/index.tsxsrcdirectorynextbuildIf this ends up being an OK approach with you @kylecarbs - I'll remove some of the extra stuff like the triangle and confetti for now before bringing in 😎