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

372 select directory channel view #384

Draft
wants to merge 7 commits into
base: 3.x
Choose a base branch
from

Conversation

joachim-n
Copy link

PR for #372.

@joachim-n
Copy link
Author

joachim-n commented Jul 5, 2024

Pushing this to get eyes on the general approach.

Still to do:

  • provide an alternative widget for the view ref field which hides the argument & count options, since we don't support them
  • check for other instances of the hardcoded view ID

@ekes
Copy link
Member

ekes commented Jul 8, 2024

So to summarise what you're doing here:

  • If a directory channel type has a field localgov_directory_channel_view (a viewfield field) the value of that field (or it's default) will be used.
  • If a directory channel type does not have the field use the same defaults as now.

Which technically seems fine. Beta module, but not an un-used beta module. It also maybe does more than we need, but could be helpful elsewhere in the future, so better to standardise. It's backward compatible. It leaves it possible to extend to a point you could make another directory channel type with/out the field, so maintains that sort of flexibility.

Which means the change to the UX is that:

(If the field exists) A content editor creating a new directory will have to select a view using viewfield to decide how the directory is displayed.

image

The proposal is to add a widget that doesn't show the advanced. The display mode is fixed by default to show only 'embed' types which makes sense. The default does show all views, which we probably don't want, but restricting them you get:

image

and

image

I think we're going to need to change the names of the view and the embed display modes here to be more user friendly. And with maybe the titles of the fields too @msayoung do you have any thoughts about this? Like how the widget could work? The reason for this is putting events into directories, which will need yet another view. So is unlike the magic we now do (swap the view based on things in the directory), the content editor creating the directory would decide how it is displayed too.

@ekes
Copy link
Member

ekes commented Jul 8, 2024

I'm going to convert this to Draft, because that's what it is, but if you can't comment on things in draft I'll swap it back. My one additional technical requirement for the todo list is: add tests ;-)

@ekes ekes marked this pull request as draft July 8, 2024 16:09
@joachim-n
Copy link
Author

Yup, the logic is:

  • a particular directory channel node can set which view & display it uses
  • if that's not set, then it falls back to a default set in the bundle info
  • if the bundle info doesn't have that set, it falls back to a universal default (which is the current hardcoded view and display)

@joachim-n
Copy link
Author

Added a test.

Still to do -- test the bundle info functionality.

@msayoung
Copy link
Member

In localgov_paragraphs we're using viewsreference so I wonder if we should use one or other module?

https://github.com/localgovdrupal/localgov_paragraphs/tree/2.x/modules/localgov_paragraphs_views

We also need to review the labels of the fields used there, which I think are also not very user friendly.

@ekes
Copy link
Member

ekes commented Oct 17, 2024

Extending the requirements to generaizing for 'Finders'*:

  • Directories wants Map and List (and I now think the map could have a different index, so that's two separate views);
  • The Consultations example needs: current and past displays of the same view;
  • News has this list, but also a separate display for promoted items.

* Finders is the name for the channel, channel items, pattern for Directories, Events, News, Blogs, Consultations...

@ekes
Copy link
Member

ekes commented Oct 17, 2024

Extending the requirements to generaizing for 'Finders'*:

* Directories wants Map and List (and I now think the map could have a different index, so that's two separate views);
* The Consultations example needs: current and past displays of the same view;
* News has this list, but also a separate display for promoted items.

* Finders is the name for the channel, channel items, pattern for Directories, Events, News, Blogs, Consultations...

Not only does it want to be able to embed them; but I realize you then want to be able to enable / disable on the content types display modes, per mode. That'll be more difficult with a multivalued field.

So maybe it wants to be a field type we control, and the names of them can be specific to the particular finders, bonus of having different config rules (what views make sense).

@joachim-n
Copy link
Author

We need more than one view display on some nodes?

@ekes
Copy link
Member

ekes commented Oct 17, 2024

We need more than one view display on some nodes?

Why not? Listing with map, just map listing, a page for current consultations, a page for past consultations...
Having them as different view display modes seems a relevant use?

@joachim-n
Copy link
Author

I wouldn't use field deltas for that use case -- too brittle. Something semantic would be better. Simplest would be just to have separate fields, one for each view.

Or -- and this might be over-engineering -- a field where we can mark each view display for its meaning.

So each value is of the form:

  • delta 0:
    -- meaning: map
    -- view_id: myview
    -- view_display: map
  • delta 1:
    -- meaning: listing
    -- view_id: myview
    -- view_display: listing

I suppose the advantage of that over the multiple fields method is that it's customizable per-node, and they can be reordered per-node. But I'm worried I might have reinvented Paragraphs :/

@rupertj
Copy link
Member

rupertj commented Oct 18, 2024

That could also have a label we can use in the UI if we're offering a choice between them. Although you could get that from the view, I guess.

@joachim-n
Copy link
Author

Do you mean that the 'meaning' field property would offer things from a dropdown? Yes, that would make sense as we'd need to have different kinds of theming or layout for them.

@ekes
Copy link
Member

ekes commented Oct 18, 2024

Are there two things here?

  1. Being able to change the display per node (channel);
  • User can decide if the event channel is in list or calendar mode
  • System automagically adds map above list on directory channel with location type content enabled
  1. Being able to change the display per content type display (per-route, per-embed)
  • Developer configures /node/{node}/map for the map view of the channel, or /node/{node}/calender for the calendar, etc. where said route could be a local task.

I think both a valid use cases. The former having the bonus of the user being able to have flexibility. The later the bonus of not loading all the things on the same page, when they might not be shown by default (which for some of this stuff is performance important).

@rupertj
Copy link
Member

rupertj commented Oct 18, 2024

I think there's three levels here:

  1. Developers need to be able to define which views should be offered to editors. This limits the choice from all views to the ones that actually work with their specific feature.

  2. Editors need to be able to define which views should be offered as a choice to users of the site. EG: "Calendar view works, but it's pointless for this content. Let's switch it off".

  3. People using the finder should be able to change the display from the ones that remain. EG: "[View as list] [View as calendar]".

@joachim-n
Copy link
Author

Yup, so I think in this MR or another I'd started something along those lines:

  • defaults views available for a node type set in code
  • admin user can edit node type config to customize / override this list
  • editor user can pick from the list for a particular node

@ekes
Copy link
Member

ekes commented Oct 18, 2024

I think you're both not including the display mode example I included?

@joachim-n
Copy link
Author

Would that not be just as easily made with two different nodes? How much content is there typically apart from the embedded view?

@ekes
Copy link
Member

ekes commented Oct 18, 2024

I'll answer this upside down

How much content is there typically apart from the embedded view?

Sometimes some, but not much, it's mainly configuration in which case the examples below could probably be done by creating routes to the views themselves, rather than using view modes. Negating my concern. So not different nodes, but using the views directly from the path.

Would that not be just as easily made with two different nodes?

But for the examples: /node/{node}/map
Desire: to have directory listing page /node/{node} with the standard full page display mode, then with a tab for a map view mode, thus not loading the map by default, but clicking through to it.
The calendar path example is much the same; but I could certainly see with the consultations example quite separate display modes for the current and past consultations.

I didn't give an example of embedded views, but could probably see that happening with news, or blog, channel nodes that are displayed within other pages.

But both node route and embed could just be directly views, directly even embedding fields from the node in the header if needed I guess.

@ekes
Copy link
Member

ekes commented Oct 21, 2024

So only question now might be: our own field from scratch, extend Viewfield that was used on this original patch, or extend Views Reference Field that Maria pointed out we are already using elsewhere?

@joachim-n
Copy link
Author

I looked at both of those modules when making this PR, as both are in my local setup. I don't remember why I picked Viewfield over the other one, but I'll have a look.

@ekes
Copy link
Member

ekes commented Oct 21, 2024

Think moving discussion onto: localgovdrupal/localgov_finders#2 makes sense.

@joachim-n
Copy link
Author

I've compared both modules and I can't see significant differences between them. Not sure why I preferred one or the other at the time -- maybe I was put off by the stuff about Paragraphs on the project page.

This MR will need to be redone anyway for Finders, and it should change to use Views Reference Field instead, since that's already in use in LGD.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants