[56339] Split content of Admin/Design page into separate tabs

[bsatarnejad]

https://community.openproject.org/work_packages/56339/activity

[HDinger]

It works as expected. I have however some remark with regards to the code structure. You are currently mixing and repeating things a bit.
When you look at the code structure of other admin pages, we usually have one file per base route (show, new, index, etc). Addtional stuff is extracted in partials. That also requires separate controllers for each module.

You have a show page, and a branding and pdf_export_styles page while all three of them are routed with the same controller. The controller actions are however exact duplicates. So there is no really benefit of doing it like that. This approach also has another drawback: In each file, you have to repeat the call of the header with a magic number passed through:

<%=
  render(Admin::DesignHeaderComponent.new(
    title: t(:"admin.custom_styles.tab_branding"),
    selected: 2,
    ))
%>

As soon as we add another tab in between, the selection will not work any more and has to be changes not only in the view files but also in the header component as well.

I am aware that you basically copied that header approach from attachments, but I think it is a bad pattern. Further, attachements has extracted stuff in individual controllers since the logic is very different.

Tl;dr; Instead of extracting all in individual files and repeating stuff, please use partials. You then only have to call the header once in the show file and can probably even put the repeated theme selector in there. The partials would then only contain the actual differences.

[EinLama]

Looks nice and does what it should, nicely done! 👍🏻

I've found one issue with the localization. It uses British English in one spot where it should be US English. Another issue regards the naming of a spec and the redirect-verification within the specs.

The other comments and suggestions are mostly code style related nitpicks and nothing functional. Please consider them and add the ones that you agree with.

[EinLama]

LGTM 🎉 great work!