BHoM / BHoM_UI

GNU Lesser General Public License v3.0
9 stars 5 forks source link

Introduce settings options within the UI framework #482

Closed FraserGreenroyd closed 6 months ago

FraserGreenroyd commented 6 months ago

NOTE: Depends on

https://github.com/BHoM/BHoM_Engine/pull/3296 - this helps make this work for testing, but is not strictly a dependency given the outline of this work.

Issues addressed by this PR

Fixes #215

Test files

There is an identically named branch on Grasshopper_UI to help test this.

image image

Changelog

Additional comments

This PR introduces the concept of settings windows for BHoM to provide greater flexibility in user set up and enhance UX across the BHoM UIs which work from this UI framework.

The work covered in this PR is NOT designed to be utilised by any UI in this milestone (7.1), as greater thought needs to be given to the individual settings each UI might want to make. For example, the ability to search when wires are dropped is a Grasshopper specific setting as Excel does not perform the same action. However, both UIs can benefit from only having certain items appear in their ctrl shift b menus based on user selection.

This PR will thus be excluded from versioning needs if changes need to be made for final implementations by others, but provides the starting blocks on which we can then aim to build. As such, perfection is not necessarily required by this PR providing it is not glaringly problematic with what's introduced, because this simply being here will not impact any user experience until explicitly called upon by a UI implementation.

FraserGreenroyd commented 6 months ago

@BHoMBot check project-compliance

bhombot-ci[bot] commented 6 months ago
@FraserGreenroyd to confirm, the following actions are now queued: - check `project-compliance`
bhombot-ci[bot] commented 6 months ago
@FraserGreenroyd fix requested for project compliance. The errors with the CSProject (`.csproj`) files have been recorded as annotations on the checks tab. I will apply the fixes to every case detailed on the checks tab with the exception of any references to the target framework. I am unable to provide fixes to the Target Framework automatically, these will need to be performed manually. If you want to perform the fixes in a different manner please resolve this manually and rerun the check. If you are happy for me to go ahead and perform this action, please reply with: > @BHoMBot fix project file ref. `21976635563`
FraserGreenroyd commented 6 months ago

@BHoMBot fix project file ref. 21976635563

bhombot-ci[bot] commented 6 months ago
@FraserGreenroyd I have queued up your request to fix the `csproj` file(s). There are 0 requests in the queue ahead of you.
bhombot-ci[bot] commented 6 months ago
@FraserGreenroyd I am now going to fix the project compliance in accordance with the annotations previously made.
bhombot-ci[bot] commented 6 months ago
@FraserGreenroyd to confirm I have now resolved the project compliance issues and pushed a commit to this Pull Request.
FraserGreenroyd commented 6 months ago

@BHoMBot check project-compliance

bhombot-ci[bot] commented 6 months ago
@FraserGreenroyd to confirm, the following actions are now queued: - check `project-compliance`
Tom-Kingstone commented 6 months ago

@BHoMBot check project-compliance @BHoMBot check core

bhombot-ci[bot] commented 6 months ago
@Tom-Kingstone to confirm, the following actions are now queued: - check `project-compliance` - check `core` There are 4 requests in the queue ahead of you.
Tom-Kingstone commented 6 months ago

@BHoMBot check copyright-compliance

bhombot-ci[bot] commented 6 months ago
@Tom-Kingstone to confirm, the following actions are now queued: - check `copyright-compliance` There are 2 requests in the queue ahead of you.
bhombot-ci[bot] commented 6 months ago
@FraserGreenroyd fix requested for copyright headers. The errors with the copyright headers on the CS (`.cs`) files have been recorded as annotations on the checks tab. I will apply the fixes to every case detailed on the checks tab. If you want to perform the fixes in a different manner please resolve this manually and rerun the check. Each CS (`.cs`) file that does not have the current copyright header will have it applied. The current copyright header can be examined [here](https://github.com/BHoM/BHoM/blob/main/COPYRIGHT_HEADER.txt). If you are happy for me to go ahead and perform this action, please reply with: > @BHoMBot fix copyright headers ref. `21992462939`
FraserGreenroyd commented 6 months ago

@BHoMBot fix copyright headers ref. 21992462939

bhombot-ci[bot] commented 6 months ago
@FraserGreenroyd I have queued up your request to fix copyright headers. There are 11 requests in the queue ahead of you.
bhombot-ci[bot] commented 6 months ago
@FraserGreenroyd I am now going to fix the copyright compliance in accordance with the annotations previously made.
bhombot-ci[bot] commented 6 months ago
@FraserGreenroyd to confirm I have now resolved the copyright compliance and pushed a commit to this Pull Request.
FraserGreenroyd commented 6 months ago

@BHoMBot check copyright-compliance

bhombot-ci[bot] commented 6 months ago
@FraserGreenroyd to confirm, the following actions are now queued: - check `copyright-compliance` There are 7 requests in the queue ahead of you.
FraserGreenroyd commented 6 months ago

@BHoMBot check project-compliance

bhombot-ci[bot] commented 6 months ago
@FraserGreenroyd to confirm, the following actions are now queued: - check `project-compliance`
FraserGreenroyd commented 6 months ago

@BHoMBot check versioning @BHoMBot check installer @BHoMBot check core

bhombot-ci[bot] commented 6 months ago
@FraserGreenroyd to confirm, the following actions are now queued: - check `versioning` - check `installer` - check `core`
FraserGreenroyd commented 6 months ago

Thanks for the review @michaelhoehn .

For item 1, I think that will need to go in as a new feature/enhancement request because that will need to update across a few parts of the search (simply adding a check in the GlobalSearch isn't enough unfortunately as there are other places it updates the search results based on a search term).

I have implemented item 2 however if you wish to take another look 😄

FraserGreenroyd commented 6 months ago

@BHoMBot check project-compliance @BHoMBot check versioning @BHoMBot check installer @BHoMBot check copyright-compliance @BHoMBot check core

bhombot-ci[bot] commented 6 months ago
@FraserGreenroyd to confirm, the following actions are now queued: - check `project-compliance` - check `versioning` - check `installer` - check `copyright-compliance` - check `core`
pawelbaran commented 6 months ago

Good stuff @FraserGreenroyd - I had not looked at the code but enjoyed the test in GH 👍 A few comments from my side:

...but of course this is all phase 2 I would say, this PR seems to be a good start!

FraserGreenroyd commented 6 months ago

Thanks for the feedback @pawelbaran 😄 To answer some points below:

  • The choice of available checkboxes looks quite random: why did AddinManagementSettings get its dedicated item? There is quite of few of such surprises, definitely worth having a closer look at reflection mechanism, a bit of curation would not hurt either (this is actually a general comment, would be great to have a thorough go through what and where is reflected in the UI - I have seen a few idiosyncrasies in the past, pretty sure we managed to grow many of them over years)

This is a good question. The bit of code responsible for handling Toolkit items is:

var toolkits = fullNames.Select(x =>
{
    var split = x.Split('.');
    if (split.Length < 3)
        return null;

    var toolkit = split[2];
    if (toolkit == "Adapters" || toolkit == "Revit" && split.Length > 3)
        toolkit = split[3];

    return toolkit;
}).Where(x => !string.IsNullOrEmpty(x)).ToList();

Which is in the LoadFromLoadedAssemblies() method for the window. I have attempted to handle adapters namespace and the Revit namespace but thinking about it, maybe this is the issue? The Revit oM namespace is presumably BH.oM.Adapters.Revit whereas other Revit assemblies may be BH.oM.Revit.AddinManagementSettings?

I agree it's one to take another look at before we turn it on fully in any of the UIs though - a workshop with a few of us to bottom out and check objects are suitable with namespaces should suffice - I'll look to schedule something in soon 😄

  • Save/load buttons would be desirable, especially as we go into more detail in the settings (see teams sharing common UX, especially valuable for newcomers)

The settings themselves load/save using the Settings_Engine which will automatically save any settings to the %ProgramData%/BHoM/Settings folder and also loaded from there on start up. Thus, sharing a setup could be done by anyone sharing the JSON settings file from that folder I think?

The Settings_Engine does need documenting better (on BHoM.xyz) to be fair, which is slated for the end of this milestone which may benefit. But equally happy to workshop around additional save/loading options - perhaps a load from file for an initial set up for people who may not be comfortable pasting the file into %ProgramData%/BHoM/Settings?

  • Discipline-specific presets would be a nice to have, I can imagine people scratching their heads under half of the items thinking whether it is relevant for their work

Agreed - I'm hoping the discipline communities can help with this 😄

  • Expert/programmer mode could also be helpful - most of people won't need namespaces such as Versioning, CSharp or Attributes

Agreed, and I think falls under point 1 above as well of checking what some names are (Attributes shouldn't be its own toolkit for example) and possible drawing up a categorisation system as well for Toolkits would be beneficial (long term strategy I think).

  • I know it is a rough prototype, but the disappearing GH component looks awkward 😃

Also agreed, which is primarily why I opted to do this PR first without doing a conjoined PR on Grasshopper, I wasn't comfortable with the approach based on the lack of buttons on ribbon option I could find so want to give that more thought/input before it becomes available. I'd also like (though it may not be feasible given resource available) to do both GH and Excel at the same time, and also, liaise with you to do Revit, so all 3 UIs can benefit potentially at once - but that needs more mapping out and workshops I think for planning week for coordination as well 😄

That said, @michaelhoehn had a reasonable idea and has documented it in this issue - I think it's worth crowding round that issue for the Grasshopper implementation specifically (and eventually the Excel one, and eventually the Revit one, and eventually the new UI one, etc., etc.).

pawelbaran commented 6 months ago

Thanks @FraserGreenroyd, all makes sense, just one immediate thought on your replies:

The settings themselves load/save using the Settings_Engine which will automatically save any settings to the %ProgramData%/BHoM/Settings folder and also loaded from there on start up. Thus, sharing a setup could be done by anyone sharing the JSON settings file from that folder I think?

What we as programmers find easy, for others may be uncomfortable or even worse, a no-go. Digging in 'system' files scares off many individuals from my experience, while everyone is fine with clicking 'Save' in the UI. So I would still opt for a button, as long as the users do not disagree 😉

FraserGreenroyd commented 6 months ago

@BHoMBot check ready-to-merge

bhombot-ci[bot] commented 6 months ago
@FraserGreenroyd to confirm, the following actions are now queued: - check `ready-to-merge`