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

Big viewer cleanup #1131

Merged
merged 11 commits into from
Oct 21, 2024
Merged

Big viewer cleanup #1131

merged 11 commits into from
Oct 21, 2024

Conversation

myieye
Copy link
Contributor

@myieye myieye commented Oct 16, 2024

This is essentially ready for review, except for the fact that it's based off of an unmerged branch.
I was finding it somewhat depressing working on viewer code, so I just went for it. There's much more to do.

  • Adds eslint to the viewer and fixes all the errors
  • Fixes svelte-check
  • Sorts options in option lists (I guess that could have been a seperate PR, but meh)
  • Refactors how options-fields work. @hahn-kev might have some objections to that one (a0086dc). It's kind of squashed in the middle...oops. The type handling deep down was very confusing for me (as well as somewhat unsafe and peculiar) and every consumer needed to handle all the mapping between options and values. I feel like I've made the components easy to use, though I did add a bit of complexity to the MultiOptionEditor and SingleOptionEditor that you might not be happy with.
  • Fixed a long standing problem for VS Code users

Some other code debt I'm eager to get to eventually:

  • Our folder structure doesn't make a ton of sense
  • I think there's a bunch of dead field-config code
  • The ProjectView component is massive

@hahn-kev
Copy link
Collaborator

Funny I was just thinking about how big the project view was. I would love to pull stuff out of that to make it smaller

@hahn-kev hahn-kev changed the base branch from develop to feat/1089-complex-forms-in-minilcm-frontend October 17, 2024 04:00
Base automatically changed from feat/1089-complex-forms-in-minilcm-frontend to develop October 17, 2024 06:13
@myieye myieye force-pushed the big-viewer-cleanup branch from 9267ef8 to a707581 Compare October 17, 2024 07:11
Copy link

github-actions bot commented Oct 17, 2024

UI unit Tests

12 tests  ±0   12 ✅ ±0   0s ⏱️ ±0s
 4 suites ±0    0 💤 ±0 
 1 files   ±0    0 ❌ ±0 

Results for commit d5d0314. ± Comparison against base commit 987d902.

♻️ This comment has been updated with latest results.

Copy link
Collaborator

@hahn-kev hahn-kev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

over all I like the changes to the Option editors. Part of why they were odd is because of how they were used before when we used FieldEditor for everything, the way the binding worked there was weird and I kinda hacked it to get it working.

I'd like to see some defaults for some of the callbacks, but I'd be happy to accept it as it is now.

@myieye myieye force-pushed the big-viewer-cleanup branch from a707581 to 6500e0d Compare October 17, 2024 07:20
@myieye myieye marked this pull request as ready for review October 18, 2024 13:47
@myieye myieye merged commit cd5ebf1 into develop Oct 21, 2024
14 checks passed
@myieye myieye deleted the big-viewer-cleanup branch October 21, 2024 10:20
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