Allow overriding preview view mode

Created on 22 June 2023, about 1 year ago
Updated 14 February 2024, 5 months ago

Since #3109560: Exempt preview view mode from being over ridden β†’ , the preview view mode has been exempt from this module's ability to override a paragraph's view mode. But there are contexts where the preview view mode is being used for something closer to a WYSIWYG feature - so it can be desirable for the preview view mode to be overridden. So can we make this configurable please?

Patch on the way :)

✨ Feature request
Status

Fixed

Version

3.0

Component

Code

Created by

πŸ‡¬πŸ‡§United Kingdom james.williams

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @james.williams
  • Issue was unassigned.
  • Status changed to Needs review about 1 year ago
  • πŸ‡¬πŸ‡§United Kingdom james.williams

    Here's a patch that adds a setting in the widget, since that seems to be where settings are kept.

    However; how does using this module with multiple form modes, and therefore multiple instances of the widget work? I haven't had a need to use the ability to bind the form mode. This patch relies on just getting the widget settings from the 'default' form mode settings for the paragraph. I can't wrap my head around whether this setting really needs to be a singleton, or what it might mean for it to be configured differently on different form modes.

    I do wonder whether a better place for the settings would be on the field instance itself, rather than the widget? But in that case, that question might apply to the other settings currently on the widget too... so is probably better dealt with in a separate issue? So I've just followed the current approach of putting the settings in the widget, and assumed this new setting would be taken from the 'default' form mode.

    One other thought - does the 'preview' view mode really need special treatment? Could other view modes exist that could be exempted? (e.g. the 'summary' view mode for paragraphs too?) In which case, could it be better to configure each view mode between being available as (1) an option to override to, (2) exempt from overriding and (3) neither (i.e. would always be overridden). This might mean changing the way the config is stored as well as configured in the UI though of course... so whilst the thought is there, I'm happy enough to just focus on configuring whether to treat the preview mode specially or not for now!

    Looking forward to hearing your thoughts! Thanks for this really handy module.

  • Status changed to Needs work 5 months ago
  • πŸ‡΅πŸ‡±Poland sayco ToruΕ„

    Hi @james.williams thank you for contributing!

    I've reviewed and tested the patch and it seems to be working fine. The code quality is very good - thank you for that.
    Before I apply the patch, could you provide the updated patch by adding the description in the configuration section of how this Apply preview feature works (what is this for)?

    So about the topics you've mentioned here.
    1. The configuration - this is something I was thinking of at the beginning of the project, so I assumed that the widget would be a better place because you might use a different configuration depending of the form mode. For example, you can have view modes: A, B, C, D, and Form modes X and Y. The X will allow you to select A and B, and you have the same component in the form mode Y which will allow you to select only C and D. I hope it makes sense. The new thing is this form mode bind, this will indeed require from developer to set things up correctly, otherwise, you won't be able to go back to the previously selected mode. Nevertheless having this on the widget level allows us to do such things.
    2. The preview mode - well this was reported by the community and AFAIK this is the view mode used for other paragraphs' ecosystem modules to preview the paragraph (not sure which - most probably the new widget for the paragraph), so I believe this one requires special treatment, but as you proposed it is wise to have the ability to turn this off. I think it is fine to that that on a widget level.

  • πŸ‡¬πŸ‡§United Kingdom james.williams

    Thanks @sayco - that's some helpful explanation. That's fine with me then to keep this issue and potential change using the existing config data structure.

    Before I apply the patch, could you provide the updated patch by adding the description in the configuration section of how this Apply preview feature works (what is this for)?

    Sorry, where do you want a description added? The $element[WidgetSettings::APPLY_TO_PREVIEW] element in the widget settings UI already has a #description. Are you suggesting that description needs improvement, or you're asking for a new description somewhere else?

  • πŸ‡΅πŸ‡±Poland sayco ToruΕ„

    I'm sorry I forgot to add "in a README.md" file, so this is where it needs to be described.

  • Status changed to Needs review 5 months ago
  • πŸ‡¬πŸ‡§United Kingdom james.williams

    How's this?

    • sayco β†’ committed 43e12775 on 3.x
      Issue #3368588 by james.williams, sayco: Allow overriding preview view...
  • πŸ‡΅πŸ‡±Poland sayco ToruΕ„

    Thanks @james.williams! The changes were fine, but I realized that I need to add some improvements to the readme anyway so I committed it along with your changes!
    This will stay in dev for some time, but it will be of course part of the next upcoming release.
    Thanks once again for contributing!

  • Status changed to Fixed 5 months ago
  • πŸ‡΅πŸ‡±Poland sayco ToruΕ„
  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.69.0 2024