UI: introduce secondary prompt content - #11987
Open
schmitz-ilias wants to merge 1 commit into
Open
Conversation
schmitz-ilias
added
kitchen sink
improvement
php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As discussed in #11105, this PR is a proposal for introducing secondary content to the show state of prompts. Secondary content is rendered in the prompt below the primary content. The primary content alone determines title and actions of the prompt. Further,
ScaleBaris taggedisPromptContent, and the implementation of the "Publish Button" in MetaData is changed accordingly to not use a roundtrip modal as prompt content anymore.I also included a small fix in the js of prompts, I kept getting an "getElementsByTagName(...).forEach not a function" error. Not sure why this isn't an issue in 11.
In contrast to what was suggested in #11105, I decided to discard actions contributed by secondary content: It should never be necessary to distribute the actions one wants to appear in the prompt over multiple components, and I also wanted to discourage doing silly things like adding multiple forms to the same prompt. If we want to be more strict about this, a bespoke interface
isSecondaryPromptContent/isNonInteractivePromptContentcould be a way to go.In the future, I could see demand for being able to also display secondary content above the primary content. Instead of being explicit about this in the signature of
Prompt\State\Factory::show, I think it would be best to decide the position of secondary content dynamically based on the component, so e.g.MessageBoxalways goes on top. In any case, this didn't seem essential for a first implementation, so for now all secondary content goes on the bottom.I would have added unit tests for this feature, but there don't seem to be any unit tests for prompts at all so far. I also haven't added a KS example. As far as I understood from the discussion in #11105, you don't want to encourage the use of
ScaleBaras prompt content, and other combinations of primary above secondary content with the availableisPromptContentcomponents also didn't seem suited for an example.Let me know what you think!
Best, Tim