Refactoring of GridViewDataSet #1548

tomasherceg · 2023-01-14T14:49:37Z

Another attempt to make GridViewDataSet extensible and loadable on the client side.

exyi

Other significant unsolved problems are:

The dataset translations will only work in the context of DataSetClientSideLoad, not in ad-hoc staticCommand
Async LoadFromQueryable (and the IPagingOptionsLoadingPostProcessor interface), I will make a separate PR for that

exyi · 2024-02-29T21:33:15Z

src/Framework/Framework/DotVVM.Framework.csproj

@@ -125,6 +128,7 @@
  <ItemGroup>
    <!-- the wildcard would not work in Target.Inputs, it only works in Include -->
    <TypescriptFile Include="Resources/Scripts/**/*.ts" />
+    <UpToDateCheckInput Include="Resources/Scripts/**/*.ts" />


What do these changes do?

src/Framework/Framework/Controls/KnockoutBindingGroup.cs

exyi · 2024-02-29T21:38:43Z

src/Framework/Framework/Controls/LinkButton.cs

+                writer.AddAttribute("onclick", KnockoutHelper.GenerateClientPostBackScript(
+                    nameof(Click), clickBinding, this,
+                    new PostbackScriptOptions(commandArgs: BindingHelper.GetParametrizedCommandArgs(this, ClickArguments))),
+                    append: true, appendSeparator: ";");


Maybe we should consider moving the postback script generation to the base class, this is somewhat arcane if someone want to make a custom button

exyi · 2024-02-29T21:39:30Z

src/Framework/Core/Controls/IRefreshableGridViewDataSet.cs

@@ -11,11 +9,11 @@ public interface IRefreshableGridViewDataSet : IBaseGridViewDataSet
        /// Gets whether the data should be refreshed. This property is set to true automatically
        /// when paging, sorting or other options change.
        /// </summary>
-        bool IsRefreshRequired { get; }
+        bool IsRefreshRequired { get; set; }

        /// <summary>
        /// Requests to reload data into the <see cref="IRefreshableGridViewDataSet" />.
        /// </summary>
        void RequestRefresh();


Do we need this method if are adding the IsRefreshRequired setter?

I actually think it's more obvious what dataset.IsRefreshRequired = true than dataset.RequestRefresh() - it's clear that it's no magic request, just a boolean flag

exyi · 2024-02-29T21:48:18Z

src/Framework/Core/Controls/GenericGridViewDataSet.cs

+        public TRowEditOptions RowEditOptions { get; set; }
+
+
+        IList IBaseGridViewDataSet.Items => Items is List<T> list ? list : Items.ToList();


I'd either throw an exception or change the return type IReadonlyList (or return a readonly list). E.g. deleting from the Items returned by this property might do nothing and I'd not want to debug this.

exyi · 2024-03-01T10:57:00Z

src/Framework/Framework/Controls/GridView.cs

+                            (dataSource as IPageableGridViewDataSet<IPagingFirstPageCapability>)?.PagingOptions.GoToFirstPage();
+                            (dataSource as IRefreshableGridViewDataSet)?.RequestRefresh();
+                        }),
+                    new IdBindingProperty($"{this.GetDotvvmUniqueId().GetValue()}_sortBinding")


We used to simply call the SortChanged without the additional paging+refresh, why is this needed? Since we now have the Button CommandParameters, we wouldn't have to touch the SortChanged binding and we could thus simply allow staticCommands here.

src/Framework/Framework/Controls/DataPager.cs

exyi · 2024-03-01T11:08:52Z

src/Framework/Framework/Controls/DataPager.cs

+            {
+                if (IsPropertySet(VisibleProperty))
+                    throw new Exception("Visible can't be set on a DataPager when HideWhenOnlyOnePage is true. You can wrap it in an element that hide that or set HideWhenOnlyOnePage to false");
+                list.SetProperty(HtmlGenericControl.VisibleProperty, hasMoreThanOnePage);


I know this error is not new, but we can combine the Visible bindings now

exyi · 2024-03-01T12:00:06Z

src/Framework/Core/Controls/Options/SortingImplementation.cs

+                if (property.GetCustomAttribute<ProtectAttribute>() is ProtectAttribute protect && protect.Settings == ProtectMode.EncryptData)
+                {
+                    throw new Exception($"Cannot sort by an property '{prop}' that is encrypted.");
+                }


We do allow sorting by private properties through?

exyi · 2024-03-01T12:08:59Z

src/Tests/ControlTests/DataPagerTests.cs

+
+            await r.RunCommand((CommandBindingExpression)nextPage.command, nextPage.control);
+            Assert.AreEqual(1, (int)r.ViewModel.Customers.PagingOptions.PageIndex);
+        }


TODO: add staticCommand datapager (just that it generates something reasonable, we have UI test for it); add AppendableDataPager

who knows if it works...

Creates a nested data context with _index and _collection extension parameters

* introduced symbolic parameters for $gridViewDataSet.loadData function. we need to substitute this loadData function on a single element, so we cannot use dotvvm-gridviewdataset to change the data context. * refactored PostbackOptions to a record to allow using `with { ... }` syntax

…ssion is multiline

…ptions

into feature/new-datasets

…tor arguments are)

…ions

exyi force-pushed the feature/new-datasets branch from 18b3058 to 2ca439b Compare October 7, 2023 11:36

tomasherceg force-pushed the feature/new-datasets branch from ac3cf9a to 8dcf71f Compare November 11, 2023 13:42

exyi force-pushed the feature/new-datasets branch from 52a010c to 65fe97f Compare November 19, 2023 20:00

tomasherceg force-pushed the feature/new-datasets branch from d57ecbb to 4c1f8c1 Compare February 25, 2024 12:13

exyi marked this pull request as ready for review February 29, 2024 21:22

exyi reviewed Mar 1, 2024

View reviewed changes

tomasherceg and others added 24 commits April 27, 2024 12:43

Refactoring of GridViewDataSet

55d7388

Fixed warnings and unit tests

64a04db

Fixed refreshing after changing the page

7c5b4de

Added prototype of JS translations for paging and sorting options

bcd1ee0

DataPager commands refactored and added support for static commands

21dbd10

Fixed JS translations APIafter rebase

04793ae

datasets: fix generating client-side reload expression

0100ac6

Add DataPager LoadData property

0520ca0

who knows if it works...

MultiCriteriaSortingOptions implemented

5bdcbf9

Sample for static command loading added

e51b450

Made DataPager work, at least with commands

94d15c7

DataPager: returned client-side data context change

69b960d

Fixed static command paging

3bacfc5

Introduce DataContextStart.CreateCollectionElement

3132d71

Creates a nested data context with _index and _collection extension parameters

Refactor DataPager to rely on Repeater, and not change its data context

69682f0

Implemented support for GridView

0d05655

Fix DataPager.RenderLinkForCurrentPage

5450478

StaticCommand Grid sample updated

16156a6

Fix DataPager.HideWhenOnlyOnePage

0460f99

Add _dataPager.Load extension parameter

20eaa0f

AppendableDataPager added

ea4264e

Uncommented commented-out samples

1af1b4f

Format knockout binding group as multiline expression when some expre…

1f71b0f

…ssion is multiline

exyi and others added 10 commits April 27, 2024 12:45

fixup! AppendableDataPager: make it work in automatic mode

0cf410a

Fixed ClickArguments to support precompilation in composite controls

912743d

Finished multi-criterion sorting

07c3d2f

Sample for consuming GitHub API

b156896

Add documentation comments to token pagers

645d97f

datasets: add DataSource original string to the generated commands IDs

fb14887

Added sample for GitHub API using static commands

a36b0bc

DataPager: disable links when not clickable (on last page or first page)

0fb573c

Revert unwanted change in JavascriptTranslatableMethodCollection

c9bacd8

datasets: Data context fixes in AppendableDataPager + some tests

3324bf6

tomasherceg force-pushed the feature/new-datasets branch from 0f3d026 to 3324bf6 Compare April 27, 2024 10:48

tomasherceg and others added 14 commits April 27, 2024 14:28

Added tests to GridViewStaticCommand page

766cbb1

datasets: return DataPager extensibility point, add ISingleRowInsertO…

c50bb49

…ptions

AppendableDataPager fixed

7ab2811

Merge branch 'feature/new-datasets' of https://github.com/riganti/dotvvm

487f86c

into feature/new-datasets

Added tests for AppendableDataPager

ca5fa35

Added tests for GitHub API test samples

9a2be8e

datasets: Remove setters from IRowEditOptions

390ba48

datasets: Add NoSorting, NoPaging and NoRowEdit options

97c8966

datasets: Make GridViewDataSetResult fields nullable (as the construc…

e88faa7

…tor arguments are)

datasets: add JS translation for getOptions

ad878e7

datasets: error handling - missing sorting capabilities

1295be1

datasets: more doccomments

882b6e7

datasets: error handling - multiple (ambiguous) interface implementat…

24fc658

…ions

datasets: accept new test outputs

4fbe3cb

exyi force-pushed the feature/new-datasets branch from a2a79a6 to 4fbe3cb Compare April 28, 2024 09:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactoring of GridViewDataSet #1548

Refactoring of GridViewDataSet #1548

tomasherceg commented Jan 14, 2023

exyi left a comment

exyi Feb 29, 2024

exyi Feb 29, 2024

exyi Feb 29, 2024

exyi Feb 29, 2024

exyi Mar 1, 2024

exyi Mar 1, 2024

exyi Mar 1, 2024

exyi Mar 1, 2024

		public TRowEditOptions RowEditOptions { get; set; }


		IList IBaseGridViewDataSet.Items => Items is List<T> list ? list : Items.ToList();

Refactoring of GridViewDataSet #1548

Are you sure you want to change the base?

Refactoring of GridViewDataSet #1548

Conversation

tomasherceg commented Jan 14, 2023

exyi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment