Skip to content
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

Add section names to settings #20

Merged
merged 1 commit into from
Aug 29, 2024

Conversation

MustafaJafar
Copy link
Contributor

@MustafaJafar MustafaJafar commented Aug 28, 2024

Changelog Description

This PR provides extra clarity about different sections in the addon settings by add section names for settings.

image

Additional info

Motivation: the wip application addon documentation is mentioning two main categories/layers for application settings.
So, I thought about reflecting that in the settings.

Testing notes:

Everything should work as usual

@MustafaJafar MustafaJafar added the type: enhancement Improvement of existing functionality or minor addition label Aug 28, 2024
@MustafaJafar MustafaJafar self-assigned this Aug 28, 2024
Copy link
Contributor

@BigRoy BigRoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For context. Here's a before screenshot.

image

I'm not completely convinced it's needed but it does look clearer at first sight so fine by me.

Maybe @Innders or @mkolar has better ideas?

@Innders
Copy link
Member

Innders commented Aug 29, 2024

I can understand the motivation behind it but I also agree with @BigRoy. I had to inspect the code to actually spot the difference.

@BigRoy
Copy link
Contributor

BigRoy commented Aug 29, 2024

I can understand the motivation behind it but I also agree with @BigRoy. I had to inspect the code to actually spot the difference.

@Innders Does that mean you approve or disapprove? Or have other comments?

@MustafaJafar
Copy link
Contributor Author

MustafaJafar commented Aug 29, 2024

I'm not completely convinced it's needed but it does look clearer at first sight so fine by me.

It's not needed. but it looks nicer in the docs.
image

@BigRoy
Copy link
Contributor

BigRoy commented Aug 29, 2024

I'd say - merge this. It's cosmetics, and doesn't affect settings much if we redesign later?

@iLLiCiTiT iLLiCiTiT merged commit 1a2b2d5 into develop Aug 29, 2024
1 check passed
@iLLiCiTiT iLLiCiTiT deleted the enhancement/add-section-names-to-settings branch August 29, 2024 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement Improvement of existing functionality or minor addition
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants