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

[FR]: Separate top and bottom bar/island margins #88

Open
atimeofday opened this issue Nov 23, 2024 · 6 comments
Open

[FR]: Separate top and bottom bar/island margins #88

atimeofday opened this issue Nov 23, 2024 · 6 comments

Comments

@atimeofday
Copy link
Contributor

Problem:
When paired with another extension which creates window gaps (Forge), the floating/island Open Bar border margin setting creates excess space below the bar, as in the attached screenshot.

image

Note:
With a bit of spare time I may be able to figure out all the pieces for a pull request, but for now, I believe this line of code refers to the (currently singular) margin slider in question:

let margin = this.createScaleWidget(0, 30, 0.2, 1, 'margin', 'Not applicable for Mainland');

Solution
Separate top/bottom margin sliders would allow me to trivially solve that issue, and hopefully be fairly easy to implement.

@neuromorph
Copy link
Owner

Hello,

This was asked once before, so I will look into it to add as a feature but may take a while as I am quite busy at the moment. Already, some new updates from GitHub are pending release.

Nevertheless, back then I had explained how to achieve this in code here. The line number may have moved a bit but the logic should be same, so you can follow that to get it working now, if you wish.

Thanks for reporting!

@atimeofday
Copy link
Contributor Author

Aha!

Thanks for pointing me to that, I had tried searching but missed both that previous request and the bit of code where the margin is actually applied. I think that probably gives me the pieces I need for a manual alteration and/or PR. I'll see what I can do!

@atimeofday
Copy link
Contributor Author

Success, probably!

I got it working locally and opened a PR #90 - it definitely needs reviewed before potentially merging, whenever you have a comfortable amount of spare time.

@DeepDoge
Copy link

DeepDoge commented Dec 3, 2024

I think instead of having a separated margin setting, having multiplier settings for top/left/bottom/right would be more flexible.
Then we can remove the hard-coded margin * 3 or margin * 1.5 values.

@atimeofday
Copy link
Contributor Author

I think instead of having a separated margin setting, having multiplier settings for top/left/bottom/right would be more flexible. Then we can remove the hard-coded margin * 3 or margin * 1.5 values.

I agree that separate top, bottom, and side (or left & right) margin settings would be best for flexibility, and I think I'll locally patch out the hard-coded multipliers for now. My three main concerns with implementing a PR were minimizing design alterations, maintaining the unified margin adjustment option, and not breaking existing configurations by replacing altered settings with new defaults.

I think I could implement the fully separate margin settings, but I don't have the experience or authority to address those three concerns while doing so. I'd be happy to learn and/or take design direction to update the PR.

@neuromorph
Copy link
Owner

Hello,

I also agree with the flexibility of separate multipliers. However, I have gone ahead with the Bottom Margin option for the reasons already mentioned by @atimeofday. Additionally, on one hand, this seems to be particularly a requirement for people using tiling to adjust the gap between the Bar and app windows. On the other hand, the settings are already overwhelming for many so I am also trying to be a bit cautious. Ideally, I would like to have few simpler options with sensible defaults and an Advanced section for each tab, maybe in the future.
Thanks for the PR, it was merged earlier. I have modified it to work with Window-Max bar and also with bar in bottom position.

This is released in the new version (OpenBar 42) out recently. Please try the latest update and let me know.

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants