OSC / ondemand

Supercomputing. Seamlessly. Open, Interactive HPC Via the Web
https://openondemand.org/
MIT License
263 stars 99 forks source link

New Help Bar Attr for ConfigurationProperty Removes All Right Side Widgets #2437

Open Oglopf opened 1 year ago

Oglopf commented 1 year ago

I'm testing out the new ConfigurationProperty object to try and document how it works and having issues understanding how it should work.

One thing that seems unexpected is no matter how I set the help_bar it removes all other widgets on the right side. Is that expected behavior? If so, is there supposed to be some new features added later to add these buttons back?

I can't see choices for the other widgets from what I gather in the ConfigurationProperty's accepted attr's it will take. This is frustrating in dev work as it will remove the restart web server button and that should always be a choice for the user.

I guess what I'm trying to understand with this issue is do we need to add more work to this object? Frankly, I've always hated the way we use environment variables as it is the wild west and very poorly documented. Having an object handle that is great, and I'd like to cram as much into it as possible and away from environment variables moving forward. But, the way this seems to be clobbering other objects in the navbar needs some saner defaults.

If someone selects help_bar we also check to make sure it doesn't just dump the rest and only gives you what that ConfigurationProperty has stored, but instead just overrides the current help menu as configured.

┆Issue is synchronized with this Asana task by Unito

Oglopf commented 1 year ago

And to be clear, the ConfigurationProperty is just a dumby singleton, and made of several classes (that needs fixed as well).

For this particular problem it's in the UserConfiguration that those attr's are being set, so looking between these two should point to what is happening.

johrstrom commented 1 year ago

One thing that seems unexpected is no matter how I set the help_bar it removes all other widgets on the right side. Is that expected behavior? If so, is there supposed to be some new features added later to add these buttons back?

Yea I think so. I mean the feature is 'let you configure it any way you want'. So it's adding anything.

I think the docs should start out with the configuration that would replicate the existing behavior. So folks have the existing help bar in YAML that they can change and iterate on.

Oglopf commented 1 year ago

Well, is it letting me define anything? I'd have to make a new object in the code that uses the ConfigurationProperty to then dynamically generate the methods which are the attrs that are used to set all this (mostly done with the private method at the bottom of UserConfiguration).

So, I guess we could set anything, it's just a high bar. But, that can be called out in the docs in the end so it's fine, but I will expect tickets on this because it took me by surprise so I'm sure it will take users by surprise.

abujeda commented 1 year ago

Not sure I understand. Currently, you do not need to change the ruby code to replicate the current help menu using the new help_bar property. Including the restart server link and currently logged-in user. I could give you an example configuration as a reference.

I am currently looking into the documentation of all the features that we have integrated. I have started with the new properties added and the profile based configuration.

I created an issue to improve the custom navigation features: https://github.com/OSC/ondemand/issues/2336 To make it easier to configure.

Oglopf commented 1 year ago

Any example configs would be great @adaybujeda.

abujeda commented 1 year ago

This is the config to replicate my local dashboard help menu.

    help_bar:
      - title: "Develop"
        links:
          - title: "Restart webserver"
            icon_uri: fa://sync
            url: "/nginx/stop?redir=/pun/dev/dashboard"
          - title: "Developer Documentation"
            icon_uri: fa://book
            url: "https://go.osu.edu/ood-app-dev"
          - title: "My Sandbox Apps"
            icon_uri: fa://cog
            url: "/pun/dev/ood/admin/dev/products"
      - title: "Help"
        links:
          - title: "Restart webserver"
            icon_uri: fa://sync
            url: "/nginx/stop?redir=/pun/dev/dashboard"
      - "log_out"
      - "user"
abujeda commented 1 year ago

I just realized that we would like to add an icon for the navigation menu items, like develop and help for example. This is currently not supported.

Will add the requirement to https://github.com/OSC/ondemand/issues/2336

Oglopf commented 1 year ago

@adaybujeda I guess that's what is confusing me, I see help_bar and assumed it was for the help but to then see the develop and help vanish is what threw me. But, I'm pretty bad at Ruby so this is starting to click seeing your example.

abujeda commented 1 year ago

We didn't know what the best name was, but help_bar defines that full right hand side navigation

Oglopf commented 1 year ago

Yeah...seeing how this works now. The name didn't click for me, but that's fine little does on this project.

So, where do these "user" and "log_out" commands come from?

abujeda commented 1 year ago

These are existing navigation templates. They are used to render the existing navigation. We created shortcuts for them, so that it was easier to configure if user wanted to add them.

We currently support these:

    all_apps: "layouts/nav/all_apps",
    featured_apps: "layouts/nav/featured_apps",
    sessions: "layouts/nav/sessions",
    log_out: "layouts/nav/log_out",
    user: "layouts/nav/user",

Navigation templates configure a full navigation item from a given alias.

Oglopf commented 1 year ago

Ok, greping through the project in dashboard I can't find where those are actually being defined.

So is the idea that those replace all the uses of render partial: 'layoutsn/nav/*' in the application.html.erb?

abujeda commented 1 year ago

The definition of the aliases for templates are here: https://github.com/OSC/ondemand/blob/master/apps/dashboard/app/apps/nav_bar.rb#L5

So is the idea that those replace all the uses of render partial: 'layoutsn/nav/*' in the application.html.erb?

Not really, this is a mechanism to re-use the existing templates in application.html.erb and simplify the configuration of a custom navigation.

Oglopf commented 1 year ago

Thanks @adaybujeda this is clicking seeing the layouts and the STATIC_LINKS hash.

abujeda commented 1 year ago

This is how the navigation and configuration looks like with the improvements from https://github.com/OSC/ondemand/issues/2336

Will raise the PR next week.

    help_bar:
      - title: "Develop"
        icon_uri: fa://code
        links:
          - title: "Restart webserver"
            icon_uri: fa://sync
            url: "restart"
          - title: "Developer Documentation"
            icon_uri: fa://book
            url: "https://go.osu.edu/ood-app-dev"
          - title: "My Sandbox Apps"
            icon_uri: fa://cog
            url: "products_dev"
      - title: "Help"
        icon_uri: fa://question-circle
        links:
          - title: "Restart webserver"
            icon_uri: fa://sync
            url: "restart"
      - "log_out"
      - "user"
Screenshot 2022-12-16 at 10 40 58