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

Multi-row functionality #928

Merged
merged 24 commits into from
Nov 8, 2024
Merged

Multi-row functionality #928

merged 24 commits into from
Nov 8, 2024

Conversation

Avid29
Copy link
Contributor

@Avid29 Avid29 commented Oct 17, 2024

Still a work in progress, but here's some screenshots:

image
image
image

@Avid29 Avid29 marked this pull request as draft October 17, 2024 04:33
@Avid29
Copy link
Contributor Author

Avid29 commented Oct 17, 2024

Remaining tasks

  • Swap to large icon when the row count is greater than 1 (see Vista)
  • Fix unlocked mode glitches (especially prominent in XP Blue based styles)
  • Remove rounded corners when unlocked (side effect of ResizingMode="CanResize").
  • Keep row count in settings
  • Adjust style properties to explicitly handle row heights instead of assuming each row doubles the taskbar height
  • Allow resizing on sides and top

(Many were hot-fixed by replacing drag functionality with a settings option)

@Avid29
Copy link
Contributor Author

Avid29 commented Oct 17, 2024

Here's every taskbar with 2 rows:

System
image

System XP
image

Watercolor
image

Windows 2000
image

Windows 95-98
image

Windows Longhorn Aero
image

Windows Me
image

Windows Vista Aero
image

Windows Vista Basic
image

Windows Vista Classic
image

Windows XP Blue
image

Windows XP Classic
image

Windows XP Embedded
image

Windows XP Olive Green
image

Windows XP Royale Noir
image

Windows XP Royale
image

Windows XP Silver
image

Windows XP Zune
image

@Avid29 Avid29 marked this pull request as ready for review October 17, 2024 07:02
@Avid29 Avid29 changed the title Resizable taskbar Multi-row functionality Oct 17, 2024
@Avid29
Copy link
Contributor Author

Avid29 commented Oct 17, 2024

Small update. The toolbar is now vertically centered (on every taskbar)

image

@dremin
Copy link
Owner

dremin commented Oct 17, 2024

Hey, thanks, this is looking great so far! I am going to try to take a deeper look at this soon.

Copy link
Owner

@dremin dremin left a comment

Choose a reason for hiding this comment

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

This is a great start! In addition to the items within the review comments, I noticed a few layout correctness issues:

  1. Within the Toolbar and the NotifyIconList controls: The WrapPanel within should have its Orientation property changed to Vertical when the taskbar is in a horizontal orientation and has more than 1 row. It should remain Horizontal otherwise.
  2. The classic Start button should not stretch vertically, it should keep its height and be aligned to the top. The XP start buttons should also be top aligned.
  3. For the XP and newer themes, the clock should change to the extended format that is used when in a vertical orientation.

I didn't pixel peep so there may be some other minor issues to address as well, but let's see how it looks after these changes! :)

RetroBar/PropertiesWindow.xaml Outdated Show resolved Hide resolved
RetroBar/Taskbar.xaml Outdated Show resolved Hide resolved
RetroBar/Taskbar.xaml.cs Outdated Show resolved Hide resolved
RetroBar/Taskbar.xaml.cs Outdated Show resolved Hide resolved
Comment on lines 966 to 967
<Setter Property="VerticalAlignment"
Value="Center"/>
Copy link
Owner

Choose a reason for hiding this comment

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

This isn't correct, the toolbars should keep their existing alignment. However, there is another related change that will need to be made for correctness, that I will comment separately on.

Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't tray icons be aligned at top as well, like quick launch?

Copy link
Owner

Choose a reason for hiding this comment

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

@1280px Yes, but that will require some more extensive theme changes.

@Avid29 Avid29 force-pushed the resizable_taskbar branch from 9ffa015 to 8b30505 Compare October 18, 2024 14:14
@Avid29 Avid29 force-pushed the resizable_taskbar branch from 8b30505 to 35844b7 Compare October 18, 2024 14:16
@Avid29
Copy link
Contributor Author

Avid29 commented Oct 18, 2024

Alignment changes made
image
image
image

@Avid29
Copy link
Contributor Author

Avid29 commented Oct 18, 2024

I added the expanded clock, and it works great for System XP and Vista, but the padding is a bit messed up (especially on XP Blue)

image
image
image

@Avid29
Copy link
Contributor Author

Avid29 commented Oct 18, 2024

I added the vertical orientation adjustment:
image
image

Based on this I have two proposals

  • Add padding
  • Flip the FlowDirection (and item order?)

@ghost
Copy link

ghost commented Oct 28, 2024

Very nice work.
I have tested this version for a week now and it is very good.

1 thing that is missing is multirow functionality for the quicklaunch items, would that be possible to add? @Avid29
now the quicklaunch items are all in 1 row,

screenshot:
aaa

EDIT:
as a suqqestion, divide the amount of quicklaunch icons by the row amount:
2 rows 10 icons would then be 5 icons on each row,
3 rows and 12 icons would be 4 icons on each row... and so on.

@Avid29
Copy link
Contributor Author

Avid29 commented Oct 29, 2024

@MKKNinetyTwo Done. Pushing now
image

@ghost
Copy link

ghost commented Oct 29, 2024

@MKKNinetyTwo Done. Pushing now image

This is perfect. Thanks!

screenshot:
aaaa

Copy link
Owner

Choose a reason for hiding this comment

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

The clock alignment is off because of the margin on the Clock style (line 1597). If you adjust this style and trigger in the same manner as ClockTemplateKey that should take care of that issue.

@@ -20,6 +20,15 @@
<s:String>Right</s:String>
<s:String>Bottom</s:String>
</x:Array>
<s:String x:Key="rowcount_text">Row Count:</s:String>
Copy link
Owner

Choose a reason for hiding this comment

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

Let's change this to Number of _rows:

@@ -203,6 +204,15 @@
SelectedIndex="{Binding Source={x:Static Settings:Settings.Instance}, Path=Edge, UpdateSourceTrigger=PropertyChanged, Converter={StaticResource enumConverter}}"
SelectionChanged="cboEdgeSelect_SelectionChanged" />
</DockPanel>
<DockPanel IsEnabled="{Binding Source={x:Static Settings:Settings.Instance}, Path=Edge, Converter={StaticResource isHorizontalConverter}}">
<Label VerticalAlignment="Center"
Target="{Binding ElementName=cboEdgeSelect}">
Copy link
Owner

Choose a reason for hiding this comment

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

The Target of this Label needs to be fixed to point to the rows ComboBox.

@dremin
Copy link
Owner

dremin commented Oct 29, 2024

Add padding

The padding and alignment of the tray icons will need some changes in a few themes. There are a few other sizing changes that will need to be made anyway. I'm okay making those minor changes later.

Flip the FlowDirection (and item order?)

I tested this and don't think it will do what we want, what are you looking to affect with this change?

@Avid29
Copy link
Contributor Author

Avid29 commented Oct 30, 2024

Flip the FlowDirection (and item order?)

I tested this and don't think it will do what we want, what are you looking to affect with this change?

I haven't tested it yet, but I expect that if the number of items in the NotifyIconsList is not divisible by the number of rows the system clock side will have empty slots were items would be while the expected behavior is that they would fill in from that side. You'd also still expect the system icons (volume, network, and such) to be on the system clock side.

@ghost
Copy link

ghost commented Nov 2, 2024

If someone wants these I made Finnish translation, and 2 custom Windows 7 themes.

These were made for Avid29 RetroBar_resizable_taskbar,
but should work on original RetroBar too.
Can be added to retrobar github if you want @dremin

Windows 7 Royale Noir (Vista theme base, Xp Royale Noir colors, Win7 startmenu):
win7_royale_noir
Windows 7 Zune (Vista theme base, Xp Zune colors, Win7 startmenu):
win7_zune

Translation and themes:
EDIT: Finnish translation download removed, see Pull Request instead Avid29#1
https://www.mediafire.com/file/zkvruzec1ujrxqq/RetroBar_Windows_7_Custom_Themes.rar

@1280px
Copy link
Contributor

1280px commented Nov 2, 2024

@MKKNinetyTwo You can submit a translation through a separate pull request :)

@ghost
Copy link

ghost commented Nov 2, 2024

@MKKNinetyTwo You can submit a translation through a separate pull request :)

I know, but the translation has lines for "row count" etc that Avid29 added, which are not yet on the main repo, Some strings are only on Avid29 repo at the moment. So I need to wait until this pull request is accepted, or if Avid29 wants to add the translation to his own repo, it is fine too.

@dremin
Copy link
Owner

dremin commented Nov 2, 2024

Flip the FlowDirection (and item order?)

I tested this and don't think it will do what we want, what are you looking to affect with this change?

I haven't tested it yet, but I expect that if the number of items in the NotifyIconsList is not divisible by the number of rows the system clock side will have empty slots were items would be while the expected behavior is that they would fill in from that side. You'd also still expect the system icons (volume, network, and such) to be on the system clock side.

Our behavior seems to match Windows XP already--the empty slots are on the clock side on both.

@Avid29
Copy link
Contributor Author

Avid29 commented Nov 2, 2024

Flip the FlowDirection (and item order?)

I tested this and don't think it will do what we want, what are you looking to affect with this change?

I haven't tested it yet, but I expect that if the number of items in the NotifyIconsList is not divisible by the number of rows the system clock side will have empty slots were items would be while the expected behavior is that they would fill in from that side. You'd also still expect the system icons (volume, network, and such) to be on the system clock side.

Our behavior seems to match Windows XP already--the empty slots are on the clock side on both.

Oh, never mind then

@ghost
Copy link

ghost commented Nov 3, 2024

Flip the FlowDirection (and item order?)

I tested this and don't think it will do what we want, what are you looking to affect with this change?

I haven't tested it yet, but I expect that if the number of items in the NotifyIconsList is not divisible by the number of rows the system clock side will have empty slots were items would be while the expected behavior is that they would fill in from that side. You'd also still expect the system icons (volume, network, and such) to be on the system clock side.

Our behavior seems to match Windows XP already--the empty slots are on the clock side on both.

"...You'd also still expect the system icons (volume, network, and such) to be on the system clock side..."

empty blocks, volume and network are on the clock side, but "removable devices" is not on clock side on retrobar. I dont know how the "removable devices" is on win xp.

@dremin
Copy link
Owner

dremin commented Nov 3, 2024

Once the requested changes are made, I have a follow up patch to enable drag resizing:

Screen.Recording.2024-11-02.at.11.14.48.PM.mov

@ghost
Copy link

ghost commented Nov 4, 2024

Once the requested changes are made,

so the remaining changes are: vertical_alignment, rowcount_text, and label target of combo_box?

btw, I noticed that the volume icon is not changing from muted to not-muted icon when clicked, I am not sure if it is retrobar bug or if it is happening only on the multirow version (running on windows 11).

- Added drag resize
- Added separate row size setting for themes
- Fixes to theme tray metrics with multiple rows
- Fixes to theme clock layout with multiple rows
- Fixed Vista language bar alignment with multiple rows
- Improved Vista and Royale-based theme backgrounds with multiple rows
- Improved classic task/toolbar spacing with multiple rows
- Addressed review comments
@dremin
Copy link
Owner

dremin commented Nov 5, 2024

I pushed a slew of changes to this branch which adds the drag resize, as well as a bunch of minor theme improvements for multi-row. I believe this is at a point it can be merged, but would appreciate others testing :)

@ghost
Copy link

ghost commented Nov 5, 2024

I pushed a slew of changes to this branch which adds the drag resize, as well as a bunch of minor theme improvements for multi-row. I believe this is at a point it can be merged, but would appreciate others testing :)

Very nice.

I made a PR for the Finnish translation if you want to add it:
Avid29#1

@Avid29
Copy link
Contributor Author

Avid29 commented Nov 6, 2024

@dremin Alright, I believe this should be ready!

Copy link
Owner

@dremin dremin left a comment

Choose a reason for hiding this comment

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

Thanks for working on this, I appreciate it!

@dremin dremin merged commit 924fcef into dremin:master Nov 8, 2024
3 checks passed
@Avid29 Avid29 deleted the resizable_taskbar branch November 8, 2024 14:28
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

Successfully merging this pull request may close these issues.

4 participants