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

RichTextField with no block elements renders with unhelpful UI components #9706

Open
dkirkham opened this issue Nov 22, 2022 · 11 comments
Open

Comments

@dkirkham
Copy link

RichTextField with no block elements renders with unhelpful UI components

This is more a cosmetic issue, than a bug per se. It might be more of a bug from an accessibility point of view.

When specifying a RichTextField without any block elements in the feature list, the widget still renders:

  • an empty menu of block elements when the circle/plus icon is clicked
  • the placeholder text suggesting the / key be used to get a list of enabled to block elements, and then an empty menu of block elements.

Both have the potentially confusing "No results" text in the menu because there are no block element types to list there.

Steps to Reproduce

  1. Start a new project
  2. Edit the HomePage model to include the field text = RichTextField(features=['bold', 'italic', 'link'])
  3. Makemigrations/migrate/runserver etc.
  4. Navigate to the rich text field in the page editor
  5. Observe the placeholder text

image

  1. Observe the menu when / is typed

image

  1. Observe the menu when the circle/plus icon is pressed

image

I consider this a bug, because when there are no block elements, the placeholder text, / and circle/plus elements should not be present. This requirement is very similar to the goals of #8249, however in my use case I still want to allow paragraphs and inline formatting; I just don't want headers, lists, etc.

  • I have confirmed that this issue can be reproduced as described on a fresh Wagtail project: (yes)

Technical details

  • Python version: 3.8.
  • Django version: 4.1.
  • Wagtail version: 4.1.1. Suspect all versions >=4.0 are affected
  • Browser version: Firefox 103, Chrome 105.0.5195.125.
@dkirkham dkirkham added status:Unconfirmed Issue, usually a bug, that has not yet been validated as a confirmed problem. type:Bug labels Nov 22, 2022
@thibaudcolas thibaudcolas self-assigned this Nov 25, 2022
@thibaudcolas thibaudcolas added component:Rich text status:Unconfirmed Issue, usually a bug, that has not yet been validated as a confirmed problem. status:Needs UI Design and removed status:Unconfirmed Issue, usually a bug, that has not yet been validated as a confirmed problem. labels Nov 25, 2022
@thibaudcolas
Copy link
Member

Thank you for the report @dkirkham! This feels like a clear UX issue. I’m not too clear on what the right fix or fixes would be for this, and whether it qualifies as a bug or not.

I agree with your plan on how to fix this, though am finding this a bit hard to reason about so any further input / feedback is very welcome.

@dkirkham
Copy link
Author

@thibaudcolas, is the issue my use case (ie. not wanting the block elements) being unreasonable, or is it deciding exactly how to present the UI in this circumstance?

Since the issue doesn’t prevent any work being done, nor does it corrupt any data or cause a crash, I’m quite happy for this not to be classified as a bug, but instead as a feature request to clean up the UI.

@thibaudcolas thibaudcolas removed the status:Unconfirmed Issue, usually a bug, that has not yet been validated as a confirmed problem. label Nov 30, 2022
@thibaudcolas
Copy link
Member

thibaudcolas commented Dec 1, 2022

Your use case is spot on, it’s the "how to present the UI in this circumstance" I’m not sure what to do with. We’re currently reviewing our implementation of the rich text UX so we’ll be looking into this aspect of it too.

I’d at least qualify it displaying "no results found" as a bug. If there are no items to display in the UI, I think I’d still prefer those UI elements to be there nonetheless, but if so they should have a separate "empty" state.

@thibaudcolas thibaudcolas assigned nicklee and unassigned thibaudcolas Jan 5, 2023
@nicklee
Copy link
Member

nicklee commented Jan 5, 2023

Definitely an interesting one, so this happens when there aren't any blocks avaliable, but the user can select things like bold/italic/link via the floating formatting bar, which is shown if they select or double click on the text they've entered.

My initial thoughts are that if there are no actions to be complete, then the user shouldn't be presented with the plus symbol. Currently this just results in a wasted click, and potential confusion due to the "no results" text.

The slight downside of removing the plus icon, will be that if users are familiar with seeing it, they then maybe confused as to why it isn't showing.

With this in mind I feel like updating the placeholder text to simply "Write something" instead of "Write something or type "/" to insert a block" when no blocks are avaliable, as well as not showing the plus symbol, would make it obvious to these users that this is intended.

So to summarise, my suggestion would be for us to do the following when no blocks are avaliable:

  • Update placeholder text to "Write something"
  • Hide the plus symbol that appears to the left of the form field

@nicklee
Copy link
Member

nicklee commented Jan 9, 2023

It seems we're agreeing that this would be a good solution. I've attached a mockup of how this would look:

page-editor_no-blocks

@nicklee nicklee assigned thibaudcolas and unassigned nicklee Jan 9, 2023
@nicklee
Copy link
Member

nicklee commented Jan 9, 2023

This has been approved to be implemented in the future

@th3hamm0r
Copy link
Contributor

+1
We hardly ever use plain TextFields, instead we use RichtTextFields with limited features, so having no blocks is quite common for us, which also resulted in some bug reports on our side since we've upgraded to wagtail versions with the new richtext UI.

@shayan-oanda
Copy link

Hi all, any progress on this?

@thibaudcolas thibaudcolas removed their assignment Nov 20, 2023
@thibaudcolas
Copy link
Member

I’d like to see this happen but have other priorities for the time being. @shayan-oanda if you want to make a pull request please go for it.

@Zogsha
Copy link

Zogsha commented Nov 21, 2023

Hello, I tried to solve this issue, but I have absolutely no idea how to do a pull request and contribute (I have no programming background)

I used the gitpod-wagtail-develop.
I just modified the getSharedPropsFromOptions in wagtail/client/src/components/Draftail/index.js and set placeholder, topToolbar and commandToolbar with a condition on blockTypes.length :

// wagtail/client/src/components/Draftail/index.js - line 209
return {
// ...
placeholder:
  blockTypes.length > 0
    ? gettext('Write something or type ‘/’ to insert a block')
    : gettext('Write something'),
// ...
topToolbar: (props) => (
  <>
    {blockTypes.length > 0 ? (
      <BlockToolbar
        {...props}
        triggerIcon={ADD_ICON}
        triggerLabel={comboBoxTriggerLabel}
        comboLabel={comboBoxLabel}
        comboPlaceholder={comboBoxLabel}
        noResultsText={gettext('No results')}
        ComboBoxComponent={ComboBox}
      />
    ) : null}
    <InlineToolbar
      {...props}
      pinButton={pinButton}
      defaultToolbar={getSavedToolbar()}
      onSetToolbar={onSetToolbar}
    />
  </>
),
// ...
commandToolbar: (props) => (
  blockTypes.length > 0 ? (
    <CommandPalette
      {...props}
      noResultsText={gettext('No results')}
      ComboBoxComponent={ComboBox}
    />
  ) : null,
// ...
};

Another solution : ComboBoxComponent={blockTypes.length > 0 ? ComboBox : null} in <BlockToolbar> and <CommandPalette>.

To demonstrate my solution, I modified a TextField in the bakerydemo into a RichTextFIeld with only inline styles. As you can see below, it looks like it works just fine.

recording-2023-11-21-14-15-22

What do you think ?

@thibaudcolas I think that another solution would be to change how <BlockToolbar>, <CommandPalette> and <DraftailEditor> from draftail handle an empty blockTypes prop.

@thibaudcolas
Copy link
Member

@Zogsha thank you for giving this a go :) Yes, the code you wrote is more or less the idea. The check needs refinement and could get quite complex as there are other things than blockTypes in there.

I’m impressed you got this far without a programming background! This specific area of Wagtail is quite hard to work on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

6 participants