-
-
Notifications
You must be signed in to change notification settings - Fork 304
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
Wizard for the Reverse Engineer Feature to improve developer UX #2626
Comments
Amazing - are you really willing to take on this huge task? This will be a great UX improvement. So we can basically add a new menu item: "Reverse Engineer Wizard (experimental)" and not break the exiting experience until we feel confident with the wizard. Some additional thoughts/concerns:
|
Yes, already started; you created a very valuable tool that my team will be relying on; glad to help. I’ll implement all of your suggestions. What do you think about an additional wizard page (post processing) for navigation properties? The page could have treeview with all selected / processed tables, containing node for navigation props. I was thinking a context menu for “rename” while keeping the F2 as well. This removes complexity of navigation properties while providing consistent interface for subsequent updates.
Get Outlook for iOS
From: Erik Ejlskov Jensen ***@***.***>Sent: Sunday, November 17, 2024 3:56 AMTo: ErikEJ/EFCorePowerTools ***@***.***>Cc: Bill Kratochvil ***@***.***>; Author ***@***.***>Subject: Re: [ErikEJ/EFCorePowerTools] Wizard for the Reverse Engineer Featureto improve developer UX (Issue #2626)
Amazing - are you really willing to take on this huge task?
This will be a great UX improvement.
So we can basically add a new menu item: "Reverse Engineer Wizard (experimental)" and not break the exiting experience until we feel confident with the wizard.
Some additional thoughts/concerns:
There is also the call to the additional "Schema selection" and "Advanced settings" dialogs, I think we can keep both as is today?
Maybe we could replace the VS statusbar feedback during dialog transitions with a dialog step giving detailed progress?
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: ***@***.***>
Get Outlook for iOS
|
I say let's stick to the basics for now .. and I do not know of any current feature for navigation renaming other than T4. |
More thoughts:
|
FYI, my plan is to focus first on functionality; once all dialogs are refactored into the wizard (and working) I will circle back and comply with all logic/UI request. So please don’t take upcoming updates as ignoring request. Another objective will be to change “as little” code as possible, while moving the code forward with its new wizard. |
@BillKrat sounds good, let's start simple! |
@ErikEJ I created a project to manage the wizard work and assigned you as admin EFCorePowerTools Reverse Engineer Wizard; please feel free to update backlog as needed. Note: I created tasked, estimated, and provided a projected roadmap for work. |
@BillKrat I added some updates to the project |
@ErikEJ reference video of wizard at work. There are two remaining development tasks in the project .
For this phase of development the goal was to touch as little code as possible. In the future should you want to upgrade the EfCorePowerTools and are interested we can revisit a pattern more suitable for wizard support (using actual BLL and DAL). I'll be adding testing tasks. This part will slow me down as I'll have to ramp up to application outside of the reverse engineer feature, e.g., untouched dialogs which should work. Since I touched very little code in the handler the scope of testing should be minimal. If you do a code compare between the current and new handlers you'll see the only changes that impact actual code (short of menu updates). I worked hard to ensure backwards compatibility. |
@ErikEJ |
@BillKrat Awsome! Potential summary wizard page: A page with a read only text box labelled "Summary" - contains the output from the Extensions output window, possibly with warning: and error: replaced with icons / emojis
Where will they be moved to?
|
On this link it was suggested they be moved to the new summary page. I'll be starting the new summary page early this week. I'll go with the guidance you provided [and any new ideas that you may have]. |
Ah! Thinking more about it and viewing your video, I think the help button / link should remain on the advanced options page, and the Rate link/button could be moved to the summary page. |
@BillKrat Just checking in if you are blocked waiting for any input from me? |
@ErikEJ Please feel free to create tasks in the project; I'll update it first opportunity and estimate the timeline. I have grandson arriving tomorrow and we'll return him early next week to have holidays with the kids. Upon our return I'll be able to prioritize this final cleanup and testing work higher. WizardWithStatusPage.mp4 |
Thanks for sharing - having a finish dialog like you show is fine - and support for Back is not needed from that. I think it is better to avoid the messsage box you dismiss in the video, and display the results (ReverseEngineerResult) that it hides in the Summary + the Rate link/button, and just keep opening the readme.md as a VS document instead of in the dialog, for much easier copy/pasting. The finish and generate buttons should be together with the back and next buttons, to keep in line with for example the "Add Entiy data Model" wizard in VS. Enjoy the time with you grandson and a happy holiday to you! |
Thanks for guidance - I'll move forward as you indicated and plumb up the existing wizard logic as well. Happy holiday to you as well! |
@BillKrat Trying to summarize the key selling points for the new wizard:
|
Good summary @ErikEJ , I had worked out the kinks of generating the code multiple times in the same session; In doing so I was able to test/fix functionality without having to start over (so perhaps not so much a stretch goal). I'll get the summary page, existing wizard launch, and status bar layout completed per your guidance. I plan to have these, as well as some technical debt, completed by Jan 1st (most likely sooner). Once you approve the completed work, I could use an assist with testing - I'm well versed with reverse engineer using SQL Client, and will test with Sqlite, but outside of that I am not familiar with the other features. |
@BillKrat "existing wizard launch, and status bar layout" The Wizard launch is out of scope for now, and not sure what you mean by status bar layout - what you have today looks great in the video. |
Happy new year @ErikEJ |
@ErikEJ with holidays over I plan on resuming this over the weekend. There is not a lot to do, just the cleanup we noted above - I'll keep you updated |
So good to be back to work so I can rest from my vacation! :) On the following link you'll find the navigation button cleanup work that I did in my last commit. I actually went back to our very original conversation where you provided me a link that has a sample of what you wanted them to look like - I emulated those. After generating files you'll see I go back so that you can see the logic applied to the button state, i.e., enabled/disabled. I could use some help on the last wizard status page. If you could please mock up what you want to see, and where I can pull it from, that would be awesome - we can wrap this up along with anything else that you see, or think of. We are real close! Video link to demonstrate wizard edited to prevent dialog from closing exposing server info in connection string. There are no popup dialogs other than the wizard. |
Looks great! Last wizard page - the internal CLI tool returns a ReverseEngineerResult object, https://github.com/ErikEJ/EFCorePowerTools/blob/master/src/GUI/Shared/Handlers/ReverseEngineer/ReverseEngineerHandler.cs#L381 This is currently listed in the VS Output pane, and many users do not see it. For first iteration, simply list the result line by line in a scrollable text box on the status screen. So basically, instead of referring to the Output window (as In your video), take advantage of the screen real estate and show everything in the status screen You foucs a lot on the schema selction screen - hardly any one uses it, only Oracle users (less than 1 % of my users) |
But all in all this looks very complete, we can iterate during a PR and after initial preview.. |
I have a kitchen pass from the wife to work on this the next couple of days [after work]; I’ll have quality time to soak in your response and git’r done. I’ll keep you posted - this is my priority now. |
EFCorePowerToolsWizardR1.mp4@ErikEJ please review and see if I captured your requirements correctly. If so, all that is left is for me to triage why the hyperlink control is trying to display within the Window (versus just in a browser); this is causing it to crash and burn. Other than that, all I have left is to update documentation to accurately reflect what was actually done. Note: code changes at 10k feet - I basically took the reverse engineer handler, made a copy of it, and used the copy (recursively) as a business logic layer for the dialog boxes. I copied/pasted the dialogs to create new wizard dialogs and used a shared view model so that state could be maintained between wizard pages. The only impacted code, outside of the new menu experimental item, should be solely in new code for the wizard and pages. When you are ready for it to go non-experimental, we just have to replace the code in the reverse engineer handler with the [experimental] copy and perform regression testing. I'm working on hyperlink issue right now, the wife and I aren't heading out for a few hours so perhaps I'll have this done before I have to call it a day. No news means I don't have resolution yet and will pick it up on Monday. |
Looks great, looking forward to the PR. Maybe I can help with the hyperlink issue during PR review? |
@ErikEJ, I have a solution which may be good enough for the experimental release (if you are okay with it). It lacks localization, as do the new wizard buttons and some status bar text, but I can resolve those before a non-experimental release. I have to head out with Teddy (dog) for his daily before the wife and I start the day, but I plan to get the sqlite toolbox link done, final look over, and a PR to you by late Monday. |
@BillKrat sounds good, when you are ready you are ready. |
@ErikEJ PR is submitted! |
EF Core Power Tools
Reverse engineering and model visualization tools for EF Core in Visual Studio 2022.
The Reverse Engineering (experimental) process aims to improve the developer UX
Reverse Engineering Wizard
Below is a 20k foot overview of the four wizard pages that comprise the wizard steps. It is beyond the scope of this
document to go into great detail, however topics that impacted the development process will be addressed.
Figure 1
Gap analysis
The goal was to make as few code changes as possible. The ReverseEngineerHandler was the main class that managed the loading of the primary view dialogs for the reverse engineering process.
Each of the three main view dialogs: PickServerDatabaseDialog, PickTableDialog, and EfCoreModalDialog,
had their own interfaces and view models. The views handled the processing and returned results that would be used
by the ReverseEngineerHandler to process subsequent views.
The path taken was to copy the ReverseEngineerHandler, name the copy RevEngWizardHandler, and provide it an interface IReverseEngineerBll [using the handlers function names]. The wizard pages would simply see the implementation of IReverseEngineerBll as an instance that can later be easily replaced. Currently, via the interface instance, the wizard pages are recursively invoking the methods on the handler - effectively reusing existing code.
To assist in maintaining state, and minimizing any code changes, the new WizardDataViewModel contains the
properties of the existing three view models, and provides an instance of a WizardEventArgs which is an EventArg
designed to maintain state, it is passed to each wizard page.
The WizardDataViewModel is shared between all wizard pages. Since all of the existing XAML (from the three view dialogs noted above) were copied to the new pages with a "wiz" prefix, it effectively allowed the views to work with the new shared view model with no real code changes.
20k foot process flow
The RevEngWizardHandler (using Microsoft's wizard infrastructure) will hit Wiz1_PickServerDatabaseDialog resulting in the OnPageVisible() function being invoked. As the [Next] button is clicked, the subsequent page OnPageVisible() will be
invoked which in turn invokes the applicable method on the implementation of the BLL instance [RevEngWizardHandler].
Note with the wiz3_EfCoreModelDialog view [in the image above] that we stray from this pattern. Instead, we invoke NextButton_Click() which in turn invokes the status page and then invokes the GenerateFiles_Click() function. This is because we initially had only the three pages [consistent with original design]. Later, we added the status page which only needed to show the results of the third page, so instead of refactoring the code, the data was set on the shared WizardDataViewModel which made it available on the last status page.
About OnPageVisible()...
In a perfect world we would be able to tap into the page loaded event and start processing. Unfortunately, like WinForms,
the WPF framework does not behave as expected once you put a load on it. e.g., the wizard worked great until we
actually requested data. Once there was a data request the UI froze (lower priority) and it wouldn't refresh until
the data processing was completed. So upon clicking "Next", the page would not advance and you wouldn't see
the helpful status bar message UNTIL the data process was complete, which wasn't very useful.
In the WinForm world we could request a DoEvents() and it would give the UI a chance to breathe (catch up); there
is an equivalent in the WPF world. Instead of duplicating the wheel, I found an awesome StatusBar control by Rick Strahl
on GitHub that did all the heavy lifting in a cool way. I added the main bits to EFCorePowerTools project under the
Westwind.WPF.Statusbar folder since it wasn't going to be a plug'n play with NuGet. This allowed me to make some minor modifications so that the UI would have a chance to breathe, move to the next page, and refresh the statusbar,
BEFORE it started the heavy lifting - the Statusbar raises an event which invokes the OnPageVisible() event. I also
tweaked Rick's code to give the UI a chance to breathe before continuing (a heavier WPF DoEvents process).
HOWEVER, great care needs to be taken that the status bar is not accessed while accessing the BLL asynchronously; we
can't be in a new thread switching back to the UI thread and have two threads using the UI thread at the same time. This is age old issue with DoEvents and WPFs underpinnings - would be nice if they provided a safe OnPageVisible()... To this
purpose you'll find I keep the code very tight when I'm invoking the BLL code asynchronously in my OnPageVisible functions. Note that the BLL cannot access the status bar (reference code in red).
EFCorePowerTools wizard framework attempts to establish a pattern that takes this heavy lifting off our plate should
additional pages be required.
Figure 2
Original Code UML
These diagrams are not all inclusive, but will provide a high level view of applicable classes and primary processes invoked.
Choose Your Data Connection (wiz pg 1)
Figure 3
Choose Your Database Objects (wiz pg 2)
Figure 4
Choose Your Settings For Project (wiz pg 3)
Figure 5
The text was updated successfully, but these errors were encountered: