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

Fix default sort order for front page card view. #5608

Closed
Tracked by #5602
stpaultim opened this issue Apr 30, 2022 · 10 comments · Fixed by backdrop/backdrop#4059
Closed
Tracked by #5602

Fix default sort order for front page card view. #5608

stpaultim opened this issue Apr 30, 2022 · 10 comments · Fixed by backdrop/backdrop#4059

Comments

@stpaultim
Copy link
Member

stpaultim commented Apr 30, 2022

Description of the bug

We have two sort order rules for the front page view of cards. We word by both date and nid. We did this at least in part because sorting by date alone was not sufficient to get the right order in Tugboat sandboxes.

Currently, if a user adds a new promoted card it shows up because of the date search, but if the date sort were to fail for some reason - a new card might not show up on the front page - because the NID sort is Ascending. @quicksketch suggested we reverse that search order and then also reverse the order in which the nodes are built.

This will not effect the display on default sites, but it will make it less likely that a user adds a new promoted card in the future that does not show up on the front page.

NOTE: If we do this, I am not sure we need the date sort at all.

This PR was requested by @quicksketch

@stpaultim
Copy link
Member Author

stpaultim commented Apr 30, 2022

I went back to the original issue to review the logic by which we decided to include both sorts and found this comment by @olafgrabienski which I think describes it.

Can we sort by date/time as first criteria and NID as a second fallback criteria for Tugboat et al? So, if the creation time is different, everything should work as expected, and people can even change the dates, if they really want to. Only if the creation time is the same for all cards, NID order steps in.
backdrop/backdrop#3862 (comment)

@stpaultim
Copy link
Member Author

stpaultim commented Apr 30, 2022

The main way to test that this is working is by adding a new card and promoting it to front page. Then disable either of the sorts and make sure that the view still shows the new Card on the front page.

Prior to this PR, if we removed the sort by date filter, the new Card would not show up.
@olafgrabienski - I would like your feedback on this.

@quicksketch - I assume that this can go in the bugfix release and does not need to be merged by Feature Freeze. Is that correct?

@olafgrabienski
Copy link

If we do this, I am not sure we need the date sort at all.
[...]
Prior to this PR, if we removed the sort by date filter, the new Card would not show up [on the home page]
olafgrabienski - I would like your feedback on this.

As you cited above, while sorting by NID is only a fallback for Tugboat sandboxes (and possibly similar environments), DATE is the main and only relevant sort criteria for standard use cases. To say it clearly, we shouldn't even think about removing the date criteria.

if the date sort were to fail for some reason - a new card might not show up on the front page - because the NID sort is Ascending

Hm, why should the date sort fail? Only reason that comes to my mind is the removal by a site architect, not very likely in my opinion. If I don't miss anything here, touching the NID based order shouldn't be necessary. Also, reversing the node building order would lead to counterintuitive numbering of the card images (first grid image: card3-organize.png, third grid image: card1-layout.png).

@stpaultim
Copy link
Member Author

stpaultim commented May 1, 2022

If I don't miss anything here, touching the NID based order shouldn't be necessary. Also, reversing the node building order would lead to counterintuitive numbering of the card images (first grid image: card3-organize.png, third grid image: card1-layout.png).

I don't feel strongly about this PR. Nate asked for this change and I created a PR for discussion, I'm also happy to leave things as they are. @olafgrabienski thanks for chiming in, I very much followed your advice in the initial settings so your feedback here is helpful.

While change the NID ordering may not be necessary, I'm not sure that there is anything intuitive about the current order that is disruptive by making the change. In this PR I renamed the numbering of the images so that:

  1. card1-layout.png is attached to node 3
  2. card2-card.png is attached to node 4
  3. card3-organize is attached to node 5

I don't see this as any more or less intuitive than it was before. I also don't think that anyone other than the author is going to care or notice which order these cards appear in. I think that if we reversed the order of the cards, no one would really notice. I like the current order, but I don't think that any specific numbering of cards is going to matter to site builders (just my opinion).

My main point is that I don't see any harm in this change and it does mean that the PR will work even if (either) one of the sort criteria are removed (as unlikely as that is).

@olafgrabienski
Copy link

I don't feel strongly about this PR.

Same here, I don't disagree with the change.

Another question while we're at it: People might ask themselves why the NID sort order is in the view. Should we set the Administrative title to something like Content: Nid [fallback for Tugboat]? (Better wording ideas welcome!)

@indigoxela
Copy link
Member

indigoxela commented May 1, 2022

Personally, I'm fine with only sorting by nid (either direction).

something like Content: Nid [fallback for Tugboat]?

I don't actually think that most people using Backdrop are aware that we use Tugboat for automated tests. Such a label might cause confusion.

@olafgrabienski
Copy link

Personally, I'm fine with only sorting by nid (either direction).

Hm, I'm confused. Only sorting by NID is definitely not the question here. A misunderstanding?

@ghost
Copy link

ghost commented May 1, 2022

I don't see the purpose of having two sort criteria. Dates could possibly be the same (especially since the default nodes are created programmatically), but NIDs will never be the same. So I'm with @indigoxela in suggesting we sort by NID and ditch the date sort field. However I don't think either direction is fine - it needs to be 'descending' so that new nodes appear as expected (the whole reason this issue was opened).

@quicksketch
Copy link
Member

I asked for the change, so I'll chime in here to say that I think backdrop/backdrop#4059 is perfect.

So I'm with @indigoxela in suggesting we sort by NID and ditch the date sort field.

I think sorting by date is preferable to NID because the value can be changed by editors. The "created" time is also modified when using the scheduled publishing. If a scheduled post is automatically published, it should be the first card at the time it's published. If we sorted by NID only, the newly published card might come in behind any cards created in the mean time.

@quicksketch
Copy link
Member

I went ahead and merged this so it's (hopefully) in its final form for the 1.22.0-preview release tomorrow.

@laryn laryn changed the title Reverse NID search order for front page card view Fix default sort order for front page card view. May 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants