Introduce Container ADRs in Decision Tab

[galuszkak]

Hi @dirkgroot @jp7677 ,

Thanks for help in previous PRs.

I'm unfamiliar with Kotlin, so I would appreciate any code review feedback.

Thanks!

[dirkgroot]

Hi @galuszkak, thanks for your PR! Could you add one or more screenshots, so we can easily see that this is going to look like in the generated site?

[galuszkak]

Hi @dirkgroot ,

those are screenshot on example workspace:

[dirkgroot]

Thanks! I think this is a good change. I think the UX could use some improvement, though. Instead of the "Container Decisions" table, I'd prefer to see a tab bar, for easier navigation.

Here's a mockup:

In this mockup, the first tab would contain Software System ADR's. Subsequent tabs would be added for container-level ADR's. Ideally, the tab bar is only shown if container-level ADR's are present.

What do you think?

[galuszkak]

@dirkgroot I really like this idea. I can try to do something for this over the weekend to do this implementation.

Any tips or suggestions on how to implement those tabs? Should I go to some inheritance for models like with SoftwareSystemPageViewModel for implementing those main tabs?
With the approach above, I can probably have one model and view class to show list decisions + tabs above them, but not sure if this is OK for You. Or should I still distinguish them in separate classes?

[dirkgroot]

@galuszkak My apologies for the delay, I've been really busy the last two weeks.

I think that an approach similar to SoftwareSystemPageViewModel would indeed be a nice way to implement this. I think all ...Decision[s]ViewModel classes could inherit from a new base view model class which contains the view model for the new tab bar. This new base view model should in turn inherit from SoftwareSystemPageViewModel.

Does this sufficiently answer your question? If not, I'm happy to help!

[galuszkak]

Hi @dirkgroot @jp7677 ,

I already had some implementation at the time of your comment, but didn't have time to finish tests so it's my first approach. Let me know if this works or I need to refactor it.
Here is screenshot how it looks like:

[dirkgroot]

Hi @galuszkak, thanks! I think your approach with this is fine. I've added some suggestions to make the code cleaner/clearer.

[galuszkak]

Hi @dirkgroot ,

Thanks for time of reviewing my PR and your insights! I've applied all of your suggestions.

[dirkgroot]

@galuszkak Cool, thanks! I've added 2 more suggestions. If those are fixed, then I think this PR is ready to be merged.

[galuszkak]

@dirkgroot added! I've also added regression test for that bug with filtering that you spotted.

[dirkgroot]

@galuszkak Approved. Thanks again for this nice contribution!