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

Convert the taxonomy listing and feed at /taxonomy/term/%term to Views #145

Open
quicksketch opened this issue Jan 5, 2014 · 14 comments
Open

Comments

@quicksketch
Copy link
Member

Sub-issue of #151

Relevant drupal.org issue: https://drupal.org/node/1857256

@ghost
Copy link

ghost commented Jan 9, 2015

Here's an initial PR that seems to work. Needs review though.

@ghost ghost added the pr - needs testing label Jan 9, 2015
@quicksketch quicksketch added this to the 1.1.0 milestone Jan 13, 2015
@quicksketch
Copy link
Member Author

This PR currently conflicts. If we can get it up and running again, now that we've branched bug fixes for 1.0.x, this can go into the 1.x branch to be included in the next minor release.

@ghost
Copy link

ghost commented Mar 3, 2015

Have updated the PR... Will wait to see test results before changing status.

@quicksketch
Copy link
Member Author

Awesome!! Yay this would be great to clean up the loose ends that are in the 1.0.x release.

@ghost
Copy link

ghost commented Mar 6, 2015

@quicksketch When replacing a hard-coded listing with a view, should the view output the same HTML as the hard-coded listing, or is it assumed that the markup will change?

Case in point: Taxonomy test has:

// Did this page request display a 'term-listing-heading'?
$this->assertPattern('|class="taxonomy-term-description"|', 'Term page displayed the term description element.');

The markup that Drupal provides, which the Taxonomy test looks for, is:

<div class="term-listing-heading"><div class="taxonomy-term vocabulary-tags" id="taxonomy-term-1">
  <div class="content">
    <div class="taxonomy-term-description"><p>Testing 123</p></div>
  </div>
</div></div>

Should I be trying to replicate those same classes/IDs in the Views output, or should I change the test to look for the new Views markup?

@quicksketch
Copy link
Member Author

I think it's a good idea to provide row-specific classes and specify classes that matched the previous ones as much as possible, but don't so far as to override a template or use a custom HTML field in Views to replicate it.

In our Node and User admin replacements, we specifically unchecked the "Provide views default classes" option as well, as there is a small performance cost in assembling all those classes.

@quicksketch quicksketch modified the milestones: 1.2.0, 1.1.0 May 10, 2015
@quicksketch
Copy link
Member Author

I closed the existing PR at backdrop/backdrop#649, as it hasn't been updated in a long time and still isn't passing tests. Looks like this is going to be bumped again to 1.3.0.

@jenlampton
Copy link
Member

Looks like this might slip again.

@jenlampton
Copy link
Member

Related #1042

@jenlampton jenlampton unassigned ghost Sep 1, 2016
@jenlampton jenlampton added this to the 1.x-future milestone Sep 1, 2016
@jenlampton
Copy link
Member

Also, I'm thinking we should put in a taxonomy/term layout with a block display view, not equivilent to the default views that were in Drupal 7. Leaving this thought here to revisit later...

@jenlampton
Copy link
Member

jenlampton commented Apr 13, 2017

After working on #2629, I've had a lot of thoughts about how to do this. Here's where I've landed, let's see what everyone thinks :)

  • Instead of providing a taxonomy view with a Page display at taxonomy/term/%, I think we should provide a Block display. (The main reason being: this will work better with layouts).

  • If the taxonomy view is enabled, then we can use the new hook added in [DX][UX] Unable to remove or replace the core taxonomy/term node listing #2629 to alter out the core node list, and add the view, using views_embed_view().

  • If someone creates a new layout to override taxonomy/term/%, the "Main page content" block will show the core node list if the view is not enabled, and the results of the view (inside the block) if it is.

  • The "Main page content" block will also contain the rendered Term entity, so if someone wants to have a page at taxonomy/term/% that shows only the view, they can remove the "Main page content" block, and drop in the "Taxonomy term list" block (or whatever we call it) instead.

  • Removing the "Main page content" block will run the risk of things not working properly (as mentioned in [DX][UX] Unable to remove or replace the core taxonomy/term node listing #2629) So we should also add an "Existing Term" block (simialr to existing content) that calls hook_entity_view().

Then we could also:

  • Provide a default layout at taxonomy/term/%
  • Place the two new blocks on the default taxonomy/term/% layout to create a more usable version of what we have now.
  • Maybe remove the hardcoded node list in favor of this layout in Backdrop 2.0?

I'm not sure how I feel about shipping core with a disabled layout (?) but if we go with the approach outlined above that last part becomes optional.

@docwilmot
Copy link
Contributor

I'm not getting the "why" of this proposal @jenlampton. If we converted taxonomy/term/% to a view, then the main content block on that page becomes that view, no? So why do all the rest?

@jenlampton
Copy link
Member

jenlampton commented Apr 14, 2017 via email

@Wylbur
Copy link
Member

Wylbur commented Dec 16, 2023

This issue really speaks to the troubles I was having when I opened #6336 and #6335! The approach that @jenlampton describes on Apr 13, 2017 addresses the heart of this issue. I'm happy to close both of those tickets in place of this issue.

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.

4 participants