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

Add "collapsibleTitle" field to modal config #2154

Closed
wants to merge 15 commits into from

Conversation

MadsBuchmann
Copy link
Contributor

@MadsBuchmann MadsBuchmann commented Mar 31, 2022

Which issue does this PR close?

This PR closes #843

What is the new behavior?

The field collapseTitle has been added to the ModalConfig type.
When that is true the title will initially be part of the content. When scrolled out of view it will appear in the toolbar.

While i was in there i took the liberty documenting the inline footer from #958.

Does this PR introduce a breaking change?

  • Yes
  • No

Checklist:

The following tasks should be carried out in sequence in order to follow the process of contributing correctly.

Reminders

  • Make sure you have implemented tests following the guidelines in: "The good: Test".
  • Make sure you have updated the cookbook with examples and showcases (for bug fixes, enhancements & new components).

Review

  • Do a self-review.
  • Request that the changes are code-reviewed
  • Request that the changes are UX reviewed (only necessary if your PR introduces visual changes)

When the pull request has been approved it will be automatically merged to stable via automerge.

dependabot bot and others added 15 commits March 24, 2022 08:51
Bumps [follow-redirects](https://github.com/follow-redirects/follow-redirects) from 1.13.3 to 1.14.7.
- [Release notes](https://github.com/follow-redirects/follow-redirects/releases)
- [Commits](follow-redirects/follow-redirects@v1.13.3...v1.14.7)

---
updated-dependencies:
- dependency-name: follow-redirects
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: RasmusKjeldgaard <rkk@bankdata.dk>
Bumps [ssri](https://github.com/npm/ssri) from 6.0.1 to 6.0.2.
- [Release notes](https://github.com/npm/ssri/releases)
- [Changelog](https://github.com/npm/ssri/blob/v6.0.2/CHANGELOG.md)
- [Commits](npm/ssri@v6.0.1...v6.0.2)

---
updated-dependencies:
- dependency-name: ssri
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: RasmusKjeldgaard <rkk@bankdata.dk>
I will have to introduce a new child element to the start of 'ion-content'. I've changed the function to search from the last element and then go to the previous sibling instead.
The modal long title feature requires that the title is inserted in both the content & the ion-header element. Therefore extend with ability to use a list and allow it to be cloned to multiple new places.
This applies the setting to every page in the modal. If we need to control it on a per page basis we should allow kirby-page-title to control it.
The previous implementation caused tests to be flakey. This does not do so in my experience.
The testing problem stemmed from the testbuilders config.component field carrying it's value across tests. Causing config.component to sometimes be null other times to be a component.
Due to ModalConfig.component having type 'any' well... Any value is valid causing the function to occasionally return null. This is not explicit in the code or properly handled. The getEmbeddedComponentElement could handle that before as it used 'lastElementSibling' & 'firstElementSibling' causing an element to always be returned as there will always be a single element (router-outlet). This has now been made explicit and is handled by both the tests and the code itself.
@MadsBuchmann MadsBuchmann changed the title Add long modal titles Add "collapsibleTitle" field to modal config Mar 31, 2022
@MadsBuchmann
Copy link
Contributor Author

MadsBuchmann commented Mar 31, 2022

Replaced by #2155... Something went wrong with the git history...

@MadsBuchmann MadsBuchmann deleted the enhancement/843-long-modal-title-rebased branch April 28, 2022 09:24
This was referenced Jun 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Enhancement] Modal title should support long titles
1 participant