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

Make lists sortable with ordering setting #3711

Merged
merged 19 commits into from Jan 30, 2020

Conversation

deanmarcussen
Copy link
Member

This fixes #2355

list-ordering

Ordering is optional and configured on the last part settings

When enabled list items are sortable, when disabled they remain sorted by created date - descending

list-part-settings

@Skrypt
Copy link
Contributor

Skrypt commented May 27, 2019

I'm confused, I'm trying to find the Vue.js component but it seems it's initialized with JQuery?

@deanmarcussen
Copy link
Member Author

I'm confused, I'm trying to find the Vue.js component but it seems it's initialized with JQuery?

Yes, I looked at the AdminMenu as you suggested, but that was done with jQuery nested sortable.
I did consider it being a Vue component, but did it this way as it meant all the js and markup could be left out of the page if ordering was disabled (as it is by default).

Figured a vue component would really suit for the other part of your notes on #2355 regarding adding existing items and detaching from #3667.

Happy to reconsider if you prefer it being done with Vue?

@Skrypt
Copy link
Contributor

Skrypt commented May 27, 2019

If the feature you are adding works it's fine merging it. I'm just surprised to see it's using JQuery instead of a full Vue.js component, but like you say it can be added later on. Up to you to do the changes on this PR or on an other one later on.

@deanmarcussen
Copy link
Member Author

Probably another PR for the other features - not totally sure how add / move / detach items would work / look like yet

@Skrypt Skrypt self-requested a review July 10, 2019 18:31
# Conflicts:
#	src/OrchardCore.Modules/OrchardCore.Lists/OrchardCore.Lists.csproj
#	src/OrchardCore.Modules/OrchardCore.Lists/Settings/ListPartSettingsDisplayDriver.cs
#	src/OrchardCore.Modules/OrchardCore.Lists/Startup.cs
@deanmarcussen
Copy link
Member Author

@Skrypt @sebastienros Fixed up some merge conflicts on this old PR.

Is it wanted?

@Skrypt
Copy link
Contributor

Skrypt commented Oct 8, 2019

Sure it is 😄
I'm just busy 😢

@agriffard
Copy link
Member

Conflicts due to Docs and Fielsets PRs that have been merged recently.
Please solve them @deanmarcussen

@agriffard
Copy link
Member

Please solve conflicts @deanmarcussen

# Conflicts:
#	src/OrchardCore.Modules/OrchardCore.Contents/Views/Content.SummaryAdmin.cshtml
#	src/OrchardCore.Modules/OrchardCore.Lists/Views/ListPartSettings.Edit.cshtml
@deanmarcussen
Copy link
Member Author

@agriffard conflicts resolved. tested as it's so old this pr, but still works fine!

@agriffard agriffard requested review from Skrypt and removed request for Skrypt December 15, 2019 14:31
@Skrypt
Copy link
Contributor

Skrypt commented Dec 15, 2019

Ok reviewing this one now.

@Skrypt
Copy link
Contributor

Skrypt commented Dec 15, 2019

It's still using netstandard 2.0 so we can't merge this.

@deanmarcussen
Copy link
Member Author

It's still using netstandard 2.0 so we can't merge this.

@Skrypt how do you figure that?

Here's the complete .csproj from that branch. On core3 as far as I can see... (note did just re sort project references)

<Project Sdk="Microsoft.NET.Sdk.Razor">

  <PropertyGroup>
    <TargetFramework>$(AspNetCoreTargetFramework)</TargetFramework>
    <AddRazorSupportForMvc>true</AddRazorSupportForMvc>
  </PropertyGroup>

  <ItemGroup>
    <FrameworkReference Include="Microsoft.AspNetCore.App" />
  </ItemGroup>

  <ItemGroup>
    <ProjectReference Include="..\..\OrchardCore\OrchardCore.Admin.Abstractions\OrchardCore.Admin.Abstractions.csproj" />
    <ProjectReference Include="..\..\OrchardCore\OrchardCore.AdminMenu.Abstractions\OrchardCore.AdminMenu.Abstractions.csproj" />
    <ProjectReference Include="..\..\OrchardCore\OrchardCore.ContentLocalization.Abstractions\OrchardCore.ContentLocalization.Abstractions.csproj" />
    <ProjectReference Include="..\..\OrchardCore\OrchardCore.ContentManagement.Abstractions\OrchardCore.ContentManagement.Abstractions.csproj" />
    <ProjectReference Include="..\..\OrchardCore\OrchardCore.ContentManagement.Display\OrchardCore.ContentManagement.Display.csproj" />
    <ProjectReference Include="..\..\OrchardCore\OrchardCore.ContentTypes.Abstractions\OrchardCore.ContentTypes.Abstractions.csproj" />
    <ProjectReference Include="..\..\OrchardCore\OrchardCore.Media.Abstractions\OrchardCore.Media.Abstractions.csproj" />
    <ProjectReference Include="..\..\OrchardCore\OrchardCore.MetaWeblog.Abstractions\OrchardCore.MetaWeblog.Abstractions.csproj" />
    <ProjectReference Include="..\..\OrchardCore\OrchardCore.Module.Targets\OrchardCore.Module.Targets.csproj" />
    <ProjectReference Include="..\..\OrchardCore\OrchardCore.Users.Abstractions\OrchardCore.Users.Abstractions.csproj" />   
    <ProjectReference Include="..\..\OrchardCore\OrchardCore.ResourceManagement.Abstractions\OrchardCore.ResourceManagement.Abstractions.csproj" />
    <ProjectReference Include="..\OrchardCore.Contents\OrchardCore.Contents.csproj" />
  </ItemGroup>

</Project>

@Skrypt
Copy link
Contributor

Skrypt commented Dec 16, 2019

I think I know what I did wrong then

@Skrypt
Copy link
Contributor

Skrypt commented Dec 16, 2019

I made some small css fixes but I can't push to your fork.

image

@Skrypt
Copy link
Contributor

Skrypt commented Dec 16, 2019

Ok it worked. The commits are pushed. I think it can be merged now.

@deanmarcussen
Copy link
Member Author

cheers @Skrypt changes LGTM. @agriffard if you feel like merging :)

@agriffard
Copy link
Member

@deanmarcussen What about a démo in today's meeting?

@deanmarcussen
Copy link
Member Author

Why not indeed Antoine :)

@@ -72,29 +59,22 @@ public override IDisplayResult Display(ListPart listPart, BuildPartDisplayContex
);
}

private async Task<PagerSlim> GetPagerAsync(IUpdateModel updater, ListPart part)
private async Task<PagerSlim> GetPagerAsync(BuildPartDisplayContext context)
Copy link
Contributor

@Skrypt Skrypt Jan 28, 2020

Choose a reason for hiding this comment

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

It should be GetPagerSlimAsync and we should also have GetPagerAsync

Copy link
Member Author

Choose a reason for hiding this comment

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

It should be GetPagerSlimAsync and we should also have GetPagerAsync

Changed to GetPagerSlimAsync.
What did you mean by also having GetPagerAsync? As a settings option too have the other type of pager?

@deanmarcussen deanmarcussen merged commit 0d46fef into OrchardCMS:dev Jan 30, 2020
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.

List of content items should be sortable.
3 participants