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 RTL support #132

Merged
merged 8 commits into from Nov 23, 2021
Merged

Add RTL support #132

merged 8 commits into from Nov 23, 2021

Conversation

xoascf
Copy link
Collaborator

@xoascf xoascf commented Oct 25, 2021

Problems (when "flow_direction" is set to RightToLeft):

  • Notification balloons do not point to the correct icon in the notification area Fixed in dff8606 (Add RTL support #132 (comment))
  • Start button may need some space on the right side (or use a space character " " in translations)
    Some original translations of themes (like Windows XP), use spaces in the text of the Start button for some RTL languages, so it's probably ok to use them as well
  • Taskbar context menu does not display correctly or update its width when switching from RTL language to LTR language
    Previously fixed in 16dc95e, now using the suggested approach in efa8823

Should be working now. Languages requiring this feature will be added soon.

Regardless of whether the language is changed, it makes sure that the right size is used for texts in all context menus of the application.
If possible, look for a better solution.
@xoascf xoascf marked this pull request as ready for review November 13, 2021 01:05
@xoascf
Copy link
Collaborator Author

xoascf commented Nov 13, 2021

@dremin, any idea how to fix the notification balloon positions for RTL languages? I haven't figured out how...

@xoascf xoascf changed the title Add dynamic flow direction Add RTL support Nov 13, 2021
@dremin
Copy link
Owner

dremin commented Nov 13, 2021

@xoascf I can take a look. Do you know how I can test this on an English installation of Windows?

@xoascf
Copy link
Collaborator Author

xoascf commented Nov 13, 2021

@xoascf I can take a look. Do you know how I can test this on an English installation of Windows?

Yeah, there should be no difference, the changes here are only visual.

@dremin dremin self-requested a review November 21, 2021 18:18
@dremin
Copy link
Owner

dremin commented Nov 21, 2021

I have balloons positioning correctly. Here are the changes I made:

  • NotifyBalloon.xaml.cs:PlacePopup needs conditions for FlowDirection for Top, Left, and Bottom edge cases
public CustomPopupPlacement[] PlacePopup(Size popupSize, Size targetSize, Point offset)
{
    CustomPopupPlacement placement;

    DpiScale dpiScale = VisualTreeHelper.GetDpi(this);

    switch ((AppBarEdge)Settings.Instance.Edge)
    {
        case AppBarEdge.Top:
            if (FlowDirection == FlowDirection.LeftToRight)
            {
                placement = new CustomPopupPlacement(new Point((popupSize.Width * -1) + (offset.X * dpiScale.DpiScaleX),
                    targetSize.Height + (offset.Y * dpiScale.DpiScaleY)),
                    PopupPrimaryAxis.Horizontal);
            }
            else
            {
                placement = new CustomPopupPlacement(new Point(-1 * offset.X * dpiScale.DpiScaleX,
                    targetSize.Height + (offset.Y * dpiScale.DpiScaleY)),
                    PopupPrimaryAxis.Horizontal);
            }
            break;
        case AppBarEdge.Left:
            placement = new CustomPopupPlacement(new Point(offset.X * dpiScale.DpiScaleX - (FlowDirection == FlowDirection.LeftToRight ? 0 : targetSize.Width),
                (popupSize.Height * -1) + (offset.Y * dpiScale.DpiScaleY)),
                PopupPrimaryAxis.Horizontal);
            break;
        case AppBarEdge.Right:
            placement = new CustomPopupPlacement(new Point((popupSize.Width * -1) + (offset.X * dpiScale.DpiScaleX),
                (popupSize.Height * -1) + (offset.Y * dpiScale.DpiScaleY)),
                PopupPrimaryAxis.Horizontal);
            break;
        default:
            // bottom taskbar
            if (FlowDirection == FlowDirection.LeftToRight)
            {
                placement = new CustomPopupPlacement(new Point((popupSize.Width * -1) + (offset.X * dpiScale.DpiScaleX),
                    (popupSize.Height * -1) + (offset.Y * dpiScale.DpiScaleY)),
                    PopupPrimaryAxis.Horizontal);
            }
            else
            {
                placement = new CustomPopupPlacement(new Point(-1 * offset.X * dpiScale.DpiScaleX,
                    (popupSize.Height * -1) + (offset.Y * dpiScale.DpiScaleY)),
                    PopupPrimaryAxis.Horizontal);
            }
            break;
    }

    return new CustomPopupPlacement[] { placement };
}
  • System.xaml: The NotifyBalloon template triggers need an additional FlowDirection condition for the Left edge case (and a corresponding one for the Right edge case) to fix arrow positions.
<ControlTemplate.Triggers>
    <MultiDataTrigger>
        <MultiDataTrigger.Conditions>
            <Condition Binding="{Binding Path=AppBarEdge, RelativeSource={RelativeSource Mode=FindAncestor, AncestorType={x:Type Window}}}"
                        Value="Left" />
            <Condition Binding="{Binding Path=FlowDirection, RelativeSource={RelativeSource Mode=FindAncestor, AncestorType={x:Type Window}}}"
                        Value="LeftToRight" />
        </MultiDataTrigger.Conditions>
        <MultiDataTrigger.Setters>
            <Setter TargetName="Arrow"
                    Property="Data"
                    Value="M 0,0 V 20 l 20,-20" />
            <Setter TargetName="Arrow"
                    Property="HorizontalAlignment"
                    Value="Left" />
        </MultiDataTrigger.Setters>
    </MultiDataTrigger>
    <MultiDataTrigger>
        <MultiDataTrigger.Conditions>
            <Condition Binding="{Binding Path=AppBarEdge, RelativeSource={RelativeSource Mode=FindAncestor, AncestorType={x:Type Window}}}"
                        Value="Right" />
            <Condition Binding="{Binding Path=FlowDirection, RelativeSource={RelativeSource Mode=FindAncestor, AncestorType={x:Type Window}}}"
                        Value="RightToLeft" />
        </MultiDataTrigger.Conditions>
        <MultiDataTrigger.Setters>
            <Setter TargetName="Arrow"
                    Property="Data"
                    Value="M 0,0 V 20 l 20,-20" />
            <Setter TargetName="Arrow"
                    Property="HorizontalAlignment"
                    Value="Left" />
        </MultiDataTrigger.Setters>
    </MultiDataTrigger>
    <DataTrigger Binding="{Binding Path=AppBarEdge, RelativeSource={RelativeSource Mode=FindAncestor, AncestorType={x:Type Window}}}"
                    Value="Top">
        <DataTrigger.Setters>
            <Setter TargetName="Border"
                    Property="DockPanel.Dock"
                    Value="Bottom" />
            <Setter TargetName="Arrow"
                    Property="Data"
                    Value="M 0,20 l 20,-20 v 20" />
            <Setter TargetName="Arrow"
                    Property="DockPanel.Dock"
                    Value="Top" />
            <Setter TargetName="Arrow"
                    Property="Margin"
                    Value="13,0,13,-2" />
        </DataTrigger.Setters>
    </DataTrigger>
</ControlTemplate.Triggers>

RetroBar/Themes/System.xaml.cs Outdated Show resolved Hide resolved
@xoascf xoascf requested a review from dremin November 23, 2021 03:05
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.

Can you remove this vestige? Once done it will be good to merge!

RetroBar/Themes/System.xaml Outdated Show resolved Hide resolved
@dremin dremin merged commit 8acf80e into dremin:master Nov 23, 2021
@xoascf xoascf deleted the dynamic-flow-direction branch November 23, 2021 06:01
@xoascf xoascf added the localization Translations or localization-related issues label Jan 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
localization Translations or localization-related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants