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

New Block: Description List #20760

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from
Open

Conversation

ellatrix
Copy link
Member

@ellatrix ellatrix commented Mar 10, 2020

Description

Fixes #4880

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/dl

This is a block that I think would have been more difficult to implement without the use of light blocks. With light blocks, we're able to use proper semantic tags in the editor, which also allow it to be styled more easily.

To do: it might be could to handle enter/delete from rich text.

Screenshot 2020-03-10 at 12 24 37

How has this been tested?

Screenshots

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@Soean
Copy link
Member

Soean commented Mar 10, 2020

Fixes #4880

@Soean Soean added the New Block Suggestion for a new block label Mar 10, 2020
Copy link
Contributor

@mcsf mcsf left a comment

Choose a reason for hiding this comment

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

Definitely cool to see how neatly we can build this now.

Now, can we make this more ergonomic? Suggestions:

  • dd: Enter twice to start a new dt (not too different from
  • dt: Space (like List blocks) to convert to dd
  • dd: (not sure about this one) Backspace to to convert to dt
  • Add transforms between those two types
  • Because DLs are visually so clean, I wonder if we need visual cues to let the user know that they are starting a new dt or dd — but what?

@ellatrix
Copy link
Member Author

Yes, I left that as a to do :D

To do: it might be could to handle enter/delete from rich text.

I think we should also take into account that you can have multiple dt elements (terms) and multiple dd elements (details). In other words, you could have multiple entries explaining a term, and you can have multiple terms for one explanation. See https://developer.mozilla.org/en-US/docs/Web/HTML/Element/dl#Multiple_terms_single_description.

Maybe we could have Enter once create a block of the same type, and Enter twice create a block of the other type. This will require more context of surrounding blocks though.

@mcsf
Copy link
Contributor

mcsf commented Mar 10, 2020

I think we should also take into account that you can have multiple dt elements (terms) and multiple dd elements (details).

Good reminder. Additionally, this to me reinforces the need for us to somehow address: "Because DLs are visually so clean, I wonder if we need visual cues to let the user know that they are starting a new dt or dd"

Maybe we could have Enter once create a block of the same type, and Enter twice create a block of the other type. This will require more context of surrounding blocks though.

Might work, yeah.

keywords: [ __( 'list' ), __( 'definitions' ), __( 'terms' ) ],
category: 'layout',
supports: {
className: false,
Copy link
Member

Choose a reason for hiding this comment

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

Why is this set to false?

For consistency, I think we should add color controls to this block if we add them to the List block.

export const settings = {
title: __( 'Description List' ),
description: __( 'List groups of terms and descriptions.' ),
keywords: [ __( 'list' ), __( 'definitions' ), __( 'terms' ) ],
Copy link
Member

Choose a reason for hiding this comment

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

I don't think list is a necessary keyword when the title already includes that word. Maybe glossary would be a better keyword to use here?


export const settings = {
title: __( 'Description List' ),
description: __( 'List groups of terms and descriptions.' ),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
description: __( 'List groups of terms and descriptions.' ),
description: __( 'List of terms and their descriptions.' ),

},
edit,
save,
attributes: {
Copy link
Member

Choose a reason for hiding this comment

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

This, the name, and the category should be moved to a block.json. (Same with other blocks being added in this PR.)

@paaljoachim
Copy link
Contributor

Quite some time has passed since there was any movement here.
What can do to help move this PR forward?

Base automatically changed from master to trunk March 1, 2021 15:43
@peroyomas
Copy link

Currently both the WHATWG and the W3C HTML5 standards support grouping dt and dd tags using the div element (link to the issue were it was first discussed), which is useful on several cases where a single concept uses more than one dt or dd tag and stuff like CSS and semantics becomes tricky. I think that would be nice to automatically wrap groups of x number of dt followed by x number of dd in div tags, even if in a list the dt and dd elements aren't directly followed by dt or dd slibings respectively.

@ellatrix
Copy link
Member Author

ellatrix commented Sep 2, 2021

Wrapping in divs is tricky, because then we somehow need to ensure that inside that div dt elements come first and the dd elements follow. If these are mixed, then there's no point to group them.

@tomdevisser
Copy link
Member

@ellatrix Are you still working on this? And could you summarize anything that's left to be done before this can be merged?

@hans2103
Copy link

Quite some time has passed since there was any movement here.
What can do to help move this PR forward?

Link to the first website ever
https://info.cern.ch/hypertext/WWW/TheProject.html
See source code... even a description list over there!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New Block Suggestion for a new block
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Block for description list
8 participants