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

Allow text wrapping in WPF controls #142

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

SebastianDevelops
Copy link

@SebastianDevelops SebastianDevelops commented Oct 22, 2024

Added the ability to wrap text by calling DiffView.EnableTextWrapping(). This addresses issue #113


  • I approached this issue by creating a Boolean value(IsTextWrapEnabled) that keeps track if the text should be wrapped or not(If DiffViewer. ToggleTextWrapping() is called).
  • If !IsTextWrapEnabled then the Add() method would simply create a StackPanel with horizontal Orientation, which does not allow text wrapping.
  • However if IsTextWrapEnabled the Add() method would create a WrapPanel instead with the same orientation as StackPanel, which does allow text wrapping.
  • I did some styling modifications for the scenario that text wraps to make sure that number,operation,text variables align within the DiffViewer.
  • To further ensure text would wrap, if IsTextWrapEnabled then we would set the Horizontal scroll bar visibility to hidden/disabled

Below is an example of the text wrapping:
https://github.com/user-attachments/assets/53d3d13e-6338-4336-92f9-a566412f1fbc

image

Copy link
Owner

@mmanela mmanela left a comment

Choose a reason for hiding this comment

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

Left comments, can you also put a high level description in the PR for how you are approaching this?

@@ -18,6 +18,7 @@
<StackPanel Orientation="Horizontal" Grid.Row="1" Background="#20808080">
<Button Width="100" Height="20" x:Name="DiffButton" Content="Switch Mode" Click="DiffButton_Click" BorderBrush="{x:Null}" BorderThickness="0" Margin="16,0,1,0" />
<Button Width="20" Height="20" x:Name="FutherActionsButton" Content="…" Click="FutherActionsButton_Click" BorderBrush="{x:Null}" BorderThickness="0" Margin="0,0,31,0" />
<Button Width="100" Height="20" x:Name="TextWrapButton" Content="WrapText" Click="TextWrapButton_Click" BorderBrush="{x:Null}" Background="Green" BorderThickness="0" Margin="16,0,1,0" />
Copy link
Owner

Choose a reason for hiding this comment

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

This should not be a button but a check box where you can enable and disable word wrapping

Copy link
Author

Choose a reason for hiding this comment

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

Pushed

@@ -59,6 +59,11 @@ private void DiffButton_Click(object sender, RoutedEventArgs e)
DiffView.ShowInline();
}

private void TextWrapButton_Click(object sender, RoutedEventArgs e)
{
DiffView.EnableTextWrapping();
Copy link
Owner

Choose a reason for hiding this comment

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

Same as above, should be a toggle box that lets you enable and disable this mode

Copy link
Author

Choose a reason for hiding this comment

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

Pushed

x:Name="SelfControl"
mc:Ignorable="d"
d:DesignHeight="450" d:DesignWidth="800">
<UserControl
Copy link
Owner

Choose a reason for hiding this comment

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

nit: This file has a lot of formatting changes, ideally, we would only include the meaningful change for review

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

@@ -41,15 +41,26 @@ internal static class Helper
internal static void RenderInlineDiffs(InternalLinesViewer viewer, ICollection<DiffPiece> lines, UIElement source, int contextLineCount)
{
viewer.Clear();
var diffViewer = source as DiffViewer;
Copy link
Owner

Choose a reason for hiding this comment

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

Shouldn't this work also for InlineDiffviewer too? If so, we can make a common interface for both to pass into this instead of UIElement.

Copy link
Author

Choose a reason for hiding this comment

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

This has been pushed

if (diffViewer.IsTextWrapEnabled)
{
var c = viewer.Add(null, null, null as string, ChangeType.Unchanged.ToString(), source);
c.Tag = line;
Copy link
Owner

Choose a reason for hiding this comment

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

This can be done once after the block with a base element for c

Copy link
Author

Choose a reason for hiding this comment

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

This has been pushed

if (diffViewer.IsTextWrapEnabled)
{
var c = viewer.Add(line.Position, "+", details, changeType.ToString(), source);
c.Tag = line;
Copy link
Owner

Choose a reason for hiding this comment

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

Same as above

Copy link
Author

Choose a reason for hiding this comment

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

This has been pushed


public WrapPanel Add(int? number, string operation, List<KeyValuePair<string, string>> value, string changeType, UIElement source)
{
if (source is DiffViewer diffViewer)
Copy link
Owner

Choose a reason for hiding this comment

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

Same comment here, we should support inlinedifferviewer and not do this check.

Copy link
Author

Choose a reason for hiding this comment

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

This has been pushed

DiffPlex.Wpf/Controls/InternalLinesViewer.xaml.cs Outdated Show resolved Hide resolved
DiffPlex.Wpf/Controls/InternalLinesViewer.xaml.cs Outdated Show resolved Hide resolved
@SebastianDevelops
Copy link
Author

@mmanela any progress on this PR?

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.

2 participants