Skip to content

Commit

Permalink
Refactor internal frame tracking & time handling (#29)
Browse files Browse the repository at this point in the history
* rework FrameTracker to consistently trigger on next frame

* use new FrameTracker implementation

* remove unnecessary difference between server & client side frame tracking

* listbox in dialog test

* prettier

* ci & cd (install .net 6 & 7)

* try CI on windows

* disable prettier check on windows CI

* fix prettier check on windows CI

* switch CI back to ubuntu-latest & stop using Task.Delay to wait on state/component changes in tests

* use TimeProvider instead of Task.Delay for transitions

* use TimeProvider instead of Task.Delay

* code format

* update tests to use TestTimeProvider

* fixes

* introduce ITimer interface

* code format

* TestTimer implementation

* fix tests
  • Loading branch information
DavidVollmers committed Sep 29, 2023
1 parent 68ee70c commit 76a3ea4
Show file tree
Hide file tree
Showing 28 changed files with 309 additions and 106 deletions.
2 changes: 1 addition & 1 deletion .gitattributes
Original file line number Diff line number Diff line change
@@ -1 +1 @@
* text=auto
* text=auto eol=lf
4 changes: 3 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ jobs:
- uses: actions/checkout@v3
- uses: actions/setup-dotnet@v3
with:
dotnet-version: 7.0.x
dotnet-version: |
6.0.x
7.0.x
- uses: actions/setup-node@v3
with:
node-version: 18.x
Expand Down
4 changes: 3 additions & 1 deletion .github/workflows/nuget-cd.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ jobs:
fetch-depth: 0
- uses: actions/setup-dotnet@v3
with:
dotnet-version: 7.0.x
dotnet-version: |
6.0.x
7.0.x
- uses: actions/setup-node@v3
with:
node-version: 18.x
Expand Down
1 change: 1 addition & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ website/Ignis.Website.Server/wwwroot/_docs
website/Ignis.Website.WebAssembly/wwwroot/_docs
website/Ignis.Website/wwwroot/js/website.min.js
website/Ignis.Website/wwwroot/css/tailwind.min.css
tests/e2e/Ignis.Tests.E2E.Website/wwwroot/css/app.min.css
40 changes: 19 additions & 21 deletions packages/Ignis.Components/FrameTracker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,38 +2,36 @@

internal class FrameTracker
{
private readonly IHostContext _hostContext;

private long _currentFrame;
private long? _frameToExecuteOn;
private IgnisComponentBase? _target;
private Action? _action;

public bool IsPending => _action != null;

public FrameTracker(IHostContext hostContext)
{
_hostContext = hostContext ?? throw new ArgumentNullException(nameof(hostContext));
}

public void ExecuteOnNextFrame(Action action, Action<bool> update)
public void ExecuteOnNextFrame(IgnisComponentBase target, Action action)
{
if (action == null) throw new ArgumentNullException(nameof(action));

// If we're server-side, we can just execute the action on the next render, otherwise we need to wait for the second render. (WebAssembly)
_action = _hostContext.IsServerSide
? action
: () =>
{
_action = action;
_target = target ?? throw new ArgumentNullException(nameof(target));
_action = action ?? throw new ArgumentNullException(nameof(action));

update(obj: false);
};
_frameToExecuteOn = _currentFrame + 1;
}

public void OnAfterRender()
{
var action = _action;
if (_currentFrame >= _frameToExecuteOn)
{
_frameToExecuteOn = null;

_action?.Invoke();

_action = null;
_action = null;
}
else if (_frameToExecuteOn.HasValue)
{
_target?.Update();
}

action?.Invoke();
++_currentFrame;
}
}
7 changes: 7 additions & 0 deletions packages/Ignis.Components/ITimer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
namespace Ignis.Components;

//TODO switch to System.Threading.ITimer when it's available

Check warning on line 3 in packages/Ignis.Components/ITimer.cs

View workflow job for this annotation

GitHub Actions / Test

TODO switch to System.Threading.ITimer when it's available (https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0026.md)

Check warning on line 3 in packages/Ignis.Components/ITimer.cs

View workflow job for this annotation

GitHub Actions / Test

TODO switch to System.Threading.ITimer when it's available (https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0026.md)

Check warning on line 3 in packages/Ignis.Components/ITimer.cs

View workflow job for this annotation

GitHub Actions / Publish

TODO switch to System.Threading.ITimer when it's available (https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0026.md)

Check warning on line 3 in packages/Ignis.Components/ITimer.cs

View workflow job for this annotation

GitHub Actions / Analyze (csharp)

TODO switch to System.Threading.ITimer when it's available (https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0026.md)

Check warning on line 3 in packages/Ignis.Components/ITimer.cs

View workflow job for this annotation

GitHub Actions / Analyze (csharp)

TODO switch to System.Threading.ITimer when it's available (https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0026.md)

Check warning on line 3 in packages/Ignis.Components/ITimer.cs

View workflow job for this annotation

GitHub Actions / Publish

TODO switch to System.Threading.ITimer when it's available (https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0026.md)

Check warning on line 3 in packages/Ignis.Components/ITimer.cs

View workflow job for this annotation

GitHub Actions / Analyze (csharp)

TODO switch to System.Threading.ITimer when it's available (https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0026.md)

Check warning on line 3 in packages/Ignis.Components/ITimer.cs

View workflow job for this annotation

GitHub Actions / Analyze (csharp)

TODO switch to System.Threading.ITimer when it's available (https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0026.md)
// https://learn.microsoft.com/en-us/dotnet/api/system.threading.itimer?view=net-8.0
internal interface ITimer : IDisposable
{
}
1 change: 1 addition & 0 deletions packages/Ignis.Components/Ignis.Components.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
<InternalsVisibleTo Include="Ignis.Components.HeadlessUI"/>
<InternalsVisibleTo Include="Ignis.Components.Reactivity"/>
<InternalsVisibleTo Include="Ignis.Components.Web"/>
<InternalsVisibleTo Include="Ignis.Tests.Common"/>
</ItemGroup>

<ItemGroup Condition="'$(TargetFramework)' == 'net6.0'">
Expand Down
2 changes: 2 additions & 0 deletions packages/Ignis.Components/IgnisComponentExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ public static IServiceCollection AddIgnis(this IServiceCollection serviceCollect

serviceCollection.TryAddScoped<IContentRegistry, ContentRegistry>();

serviceCollection.TryAddSingleton<TimeProvider, TimeProviderImplementation>();

return serviceCollection;
}

Expand Down
11 changes: 11 additions & 0 deletions packages/Ignis.Components/TimeProvider.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
namespace Ignis.Components;

//TODO switch to System.TimeProvider when it's available

Check warning on line 3 in packages/Ignis.Components/TimeProvider.cs

View workflow job for this annotation

GitHub Actions / Test

Check warning on line 3 in packages/Ignis.Components/TimeProvider.cs

View workflow job for this annotation

GitHub Actions / Test

Check warning on line 3 in packages/Ignis.Components/TimeProvider.cs

View workflow job for this annotation

GitHub Actions / Publish

Check warning on line 3 in packages/Ignis.Components/TimeProvider.cs

View workflow job for this annotation

GitHub Actions / Analyze (csharp)

Check warning on line 3 in packages/Ignis.Components/TimeProvider.cs

View workflow job for this annotation

GitHub Actions / Analyze (csharp)

Check warning on line 3 in packages/Ignis.Components/TimeProvider.cs

View workflow job for this annotation

GitHub Actions / Publish

Check warning on line 3 in packages/Ignis.Components/TimeProvider.cs

View workflow job for this annotation

GitHub Actions / Analyze (csharp)

Check warning on line 3 in packages/Ignis.Components/TimeProvider.cs

View workflow job for this annotation

GitHub Actions / Analyze (csharp)

// https://learn.microsoft.com/en-us/dotnet/api/system.timeprovider?view=net-8.0
internal abstract class TimeProvider
{
public virtual ITimer CreateTimer(TimerCallback callback, object? state, TimeSpan dueTime, TimeSpan period)
{
return new TimerImplementation(callback, state, dueTime, period);
}
}
5 changes: 5 additions & 0 deletions packages/Ignis.Components/TimeProviderImplementation.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
namespace Ignis.Components;

internal class TimeProviderImplementation : TimeProvider
{
}
16 changes: 16 additions & 0 deletions packages/Ignis.Components/TimerImplementation.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
namespace Ignis.Components;

internal class TimerImplementation : ITimer
{
private readonly Timer _timer;

public TimerImplementation(TimerCallback callback, object? state, TimeSpan dueTime, TimeSpan period)
{
_timer = new Timer(callback, state, dueTime, period);
}

public void Dispose()
{
_timer.Dispose();
}
}
6 changes: 3 additions & 3 deletions packages/Tailwind/Ignis.Components.HeadlessUI/Dialog.cs
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ public void Open(Action? continueWith = null)

var __ = IsOpenChanged.InvokeAsync(_isOpen = true);

if (continueWith != null) FrameTracker.ExecuteOnNextFrame(continueWith, Update);
if (continueWith != null) FrameTracker.ExecuteOnNextFrame(this, continueWith);

Update();
}
Expand All @@ -206,7 +206,7 @@ private void CloseCore(Action? continueWith, bool async = false)
{
var __ = IsOpenChanged.InvokeAsync(_isOpen = false);

if (continueWith != null) FrameTracker.ExecuteOnNextFrame(continueWith, Update);
if (continueWith != null) FrameTracker.ExecuteOnNextFrame(this, continueWith);

Update(async);
}
Expand All @@ -226,7 +226,7 @@ public void SetDescription(IDialogDescription description)
/// <inheritdoc />
public void CloseFromTransition(Action? continueWith = null)
{
CloseCore(continueWith, true);
CloseCore(continueWith, async: true);
}

/// <inheritdoc />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ public void Open(Action? continueWith = null)
_ = IsOpenChanged.InvokeAsync(_isOpen = true);

if (_transition != null)
FrameTracker.ExecuteOnNextFrame(() => _transition.Show(() => OnAfterOpen(continueWith)), Update);
else if (continueWith != null) FrameTracker.ExecuteOnNextFrame(() => OnAfterOpen(continueWith), Update);
FrameTracker.ExecuteOnNextFrame(this, () => _transition.Show(() => OnAfterOpen(continueWith)));
else if (continueWith != null) FrameTracker.ExecuteOnNextFrame(this, () => OnAfterOpen(continueWith));

Update();
}
Expand Down Expand Up @@ -73,7 +73,7 @@ private void CloseCore(Action? continueWith, bool async = false)
{
_ = IsOpenChanged.InvokeAsync(_isOpen = false);

if (continueWith != null) FrameTracker.ExecuteOnNextFrame(continueWith, Update);
if (continueWith != null) FrameTracker.ExecuteOnNextFrame(this, continueWith);

Update(async);
}
Expand Down
7 changes: 3 additions & 4 deletions packages/Tailwind/Ignis.Components.HeadlessUI/Transition.cs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ public bool Show

[Parameter] public bool Appear { get; set; }

/// <inheritdoc />
[CascadingParameter] public IContentHost? Outlet { get; set; }

[CascadingParameter] public IMenu? Menu { get; set; }
Expand Down Expand Up @@ -174,7 +173,7 @@ protected override void LeaveTransition(Action? continueWith = null)
{
_transitioningTo = false;

WatchTransition(false, () =>
WatchTransition(isEnter: false, () =>
{
_show = false;
Expand Down Expand Up @@ -221,7 +220,7 @@ private void WatchTransition(bool isEnter, Action? continueWith)
var startedTransitions = new List<ITransitionChild>();
var finishedTransitions = 0;

if (isEnter) base.EnterTransition(() => AggregateDialogs(true, ContinueWith));
if (isEnter) base.EnterTransition(() => AggregateDialogs(open: true, ContinueWith));
else ContinueWith();
return;

Expand All @@ -243,7 +242,7 @@ void ContinueWith()
if (finishedTransitions == startedTransitions.Count + 1)
{
if (isEnter) continueWith?.Invoke();
else AggregateDialogs(false, () => base.LeaveTransition(continueWith));
else AggregateDialogs(open: false, () => base.LeaveTransition(continueWith));
}
}
}
Expand Down
49 changes: 32 additions & 17 deletions packages/Tailwind/Ignis.Components.HeadlessUI/TransitionBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ public abstract class TransitionBase : IgnisComponentBase, ICssClass, IHandleAft
{
get
{
var originalClassString = AdditionalAttributes?.FirstOrDefault(a => string.Equals(a.Key, "class", StringComparison.OrdinalIgnoreCase));
var originalClassString = AdditionalAttributes?.FirstOrDefault(a =>
string.Equals(a.Key, "class", StringComparison.OrdinalIgnoreCase));
return _state switch
{
TransitionState.Entering => $"{originalClassString?.Value} {Enter} {EnterFrom}".Trim(),
Expand Down Expand Up @@ -68,6 +69,8 @@ public abstract class TransitionBase : IgnisComponentBase, ICssClass, IHandleAft

[Inject] internal FrameTracker FrameTracker { get; set; } = null!;

[Inject] internal TimeProvider TimeProvider { get; set; } = null!;

protected virtual void EnterTransition(Action? continueWith = null)
{
if (_state != TransitionState.Default && _state != TransitionState.CanEnter) return;
Expand All @@ -76,17 +79,22 @@ protected virtual void EnterTransition(Action? continueWith = null)

UpdateState(TransitionState.Entering, () =>
{
ITimer timer = null!;
var (graceDuration, transitionDuration) = ParseDuration(Enter);
_ = Task.Delay(graceDuration).ContinueWith(__ =>
timer = TimeProvider.CreateTimer(_ =>
{
// ReSharper disable once AccessToModifiedClosure
timer.Dispose();
UpdateState(TransitionState.Entered, () =>
{
_ = Task.Delay(transitionDuration).ContinueWith(_ =>
timer = TimeProvider.CreateTimer(_ =>
{
// ReSharper disable once AccessToModifiedClosure
timer.Dispose();
UpdateState(TransitionState.CanLeave, continueWith);
});
}, state: null, transitionDuration, Timeout.InfiniteTimeSpan);
});
});
}, state: null, graceDuration, Timeout.InfiniteTimeSpan);
});
}

Expand All @@ -98,19 +106,25 @@ protected virtual void LeaveTransition(Action? continueWith = null)

UpdateState(TransitionState.Leaving, () =>
{
ITimer timer = null!;
var (graceDuration, transitionDuration) = ParseDuration(Leave);
_ = Task.Delay(graceDuration).ContinueWith(__ =>
timer = TimeProvider.CreateTimer(_ =>
{
// ReSharper disable once AccessToModifiedClosure
timer.Dispose();
UpdateState(TransitionState.Left, () =>
{
_ = Task.Delay(transitionDuration).ContinueWith(_ =>
timer = TimeProvider.CreateTimer(_ =>
{
// ReSharper disable once AccessToModifiedClosure
timer.Dispose();
RenderContent = false;
UpdateState(TransitionState.CanEnter, continueWith);
});
}, state: null, transitionDuration, Timeout.InfiniteTimeSpan);
});
});
}, state: null, graceDuration, Timeout.InfiniteTimeSpan);
});
}

Expand All @@ -120,9 +134,9 @@ private void UpdateState(TransitionState state, Action? continueWith)

_state = state;

if (continueWith != null) FrameTracker.ExecuteOnNextFrame(continueWith, Update);
if (continueWith != null) FrameTracker.ExecuteOnNextFrame(this, continueWith);

Update(true);
Update(async: true);
}

/// <inheritdoc />
Expand All @@ -133,12 +147,12 @@ public virtual Task OnAfterRenderAsync()
return Task.CompletedTask;
}

private static (int, int) ParseDuration(string? classString)
private static (TimeSpan, TimeSpan) ParseDuration(string? classString)
{
var durationClass = classString?.Split(' ')
.Select(v => v.Trim().Split(':')[v.Trim().Split(':').Length - 1])
.FirstOrDefault(v => v.StartsWith("duration-", StringComparison.Ordinal));
if (durationClass == null) return (0, 0);
if (durationClass == null) return (TimeSpan.Zero, TimeSpan.Zero);

var factor = 1;

Expand All @@ -156,15 +170,16 @@ private static (int, int) ParseDuration(string? classString)
durationString = durationString[..^1];
factor = 1000;
}
else return (0, 0);
else return (TimeSpan.Zero, TimeSpan.Zero);
}

var duration = int.Parse(durationString, CultureInfo.InvariantCulture) * factor;
if (duration <= 0) return (0, 0);
if (duration <= 0) return (TimeSpan.Zero, TimeSpan.Zero);

return duration < TransitionGraceDuration
? (0, duration)
: (TransitionGraceDuration, duration - TransitionGraceDuration);
? (TimeSpan.Zero, TimeSpan.FromMilliseconds(duration))
: (TimeSpan.FromMilliseconds(TransitionGraceDuration),
TimeSpan.FromMilliseconds(duration - TransitionGraceDuration));
}

private enum TransitionState
Expand Down
18 changes: 18 additions & 0 deletions tests/Ignis.Tests.Common/IgnisTestExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
using Ignis.Components;
using Microsoft.Extensions.DependencyInjection;

namespace Ignis.Tests.Common;

public static class IgnisTestExtensions
{
public static IServiceCollection AddIgnisTestServices(this IServiceCollection serviceCollection)
{
if (serviceCollection == null) throw new ArgumentNullException(nameof(serviceCollection));

serviceCollection.AddIgnis();
serviceCollection.AddSingleton<IHostContext, TestHostContext>();
serviceCollection.AddSingleton<TimeProvider, TestTimeProvider>();

return serviceCollection;
}
}
2 changes: 1 addition & 1 deletion tests/Ignis.Tests.Common/TestHostContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

namespace Ignis.Tests.Common;

public class TestHostContext : IHostContext
internal class TestHostContext : IHostContext
{
public bool IsPrerendering => false;

Expand Down
11 changes: 11 additions & 0 deletions tests/Ignis.Tests.Common/TestTimeProvider.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
using Ignis.Components;

namespace Ignis.Tests.Common;

internal class TestTimeProvider : TimeProvider
{
public override ITimer CreateTimer(TimerCallback callback, object? state, TimeSpan dueTime, TimeSpan period)
{
return new TestTimer(callback);
}
}

0 comments on commit 76a3ea4

Please sign in to comment.