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

[Enhancement] Modal title should support long titles #843

Closed
bommariusser opened this issue Dec 19, 2019 · 15 comments
Closed

[Enhancement] Modal title should support long titles #843

bommariusser opened this issue Dec 19, 2019 · 15 comments
Assignees
Labels

Comments

@bommariusser
Copy link
Contributor

bommariusser commented Dec 19, 2019

Is your enhancement request related to a problem? Please describe.
Right now our modals truncate on long titles, which makes some important text go away.

Describe the solution you'd like
We would like the text to truncate only on scrolling down - So it will show the whole text when opening a modal.

Skærmbillede 2019-12-19 kl  12 10 25

Describe alternatives you've considered
We also thought of showing the whole title all the time, but prefer truncating on scrolling, like our page-template.

Also we have thought about scaling the title down to a p / 16 px before making the header bigger, down to the same size as the toolbar title.

Skærmbillede 2021-03-08 kl  18 35 01

@bommariusser bommariusser added enhancement New feature or request component:Modal labels Dec 19, 2019
@bommariusser bommariusser added this to the Milestone 1 - 2020 milestone Jan 29, 2020
@labanos labanos added the NOT Tech refined Needs Tech kickoff - solution outlined and agreed label Feb 28, 2020
@bommariusser bommariusser added the good first issue Good for newcomers label Mar 3, 2021
@jakobe jakobe removed this from the Milestone 1 - 2020 milestone Mar 8, 2021
@flemcito
Copy link
Contributor

flemcito commented Mar 9, 2021

Tech refinement:
The modal header uses the ion-toolbar, which is quite limited with respect to what can be adjusted. Unfortunately the props that control height/overflow behaviour are not exposed. https://ionicframework.com/docs/api/toolbar.

  • One way forward could be to replace the ion-toolbar with a custom component, but I am unsure of the implications this would have (ie. risk of breaking changes).
  • Another way could be to create a variant of the modal with the collapsible header and make it an explicit choice to opt into that behaviour - so not to risk breaking existing implementations.
  • A third option could be to place the modal heading in the content area, and only use the toolbar to show a trunctacted heading when scrolled (like we do on pages)

@MartinSkovsen
Copy link
Contributor

MartinSkovsen commented Mar 9, 2021

I'll add to this by refering to recently merged PR #1217, which displays how the toolbar is currently being used. #1355, not-tech-refined issue, also has some relevance to this.

@jakobe jakobe removed the good first issue Good for newcomers label Mar 25, 2021
@flemcito flemcito removed their assignment May 25, 2021
@stale
Copy link

stale bot commented Aug 3, 2021

This issue has been automatically marked as stale because of no recent activity. It will be closed in 10 weeks if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Automatically applied when there is no activity on an issue or PR label Aug 3, 2021
@LVinther
Copy link

@henrikvoetmand we need this enhancement for all our "Samtykker/aftaler" (across DRB) so we need to prioritize this issue 😄

@stale stale bot removed the stale Automatically applied when there is no activity on an issue or PR label Aug 11, 2021
@stale
Copy link

stale bot commented Oct 22, 2021

This issue has been automatically marked as stale because of no recent activity. It will be closed in 10 weeks if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Automatically applied when there is no activity on an issue or PR label Oct 22, 2021
@LVinther
Copy link

LVinther commented Oct 24, 2021

@henrikvoetmand as mentioned before this issue is important for all our "Samtykker/aftaler" but it is now 'Stale'. Is it possible to prioritize this issue? :)

@stale stale bot removed the stale Automatically applied when there is no activity on an issue or PR label Oct 24, 2021
@henrikvoetmand
Copy link
Collaborator

@alxzak can we prioritize this issue?

@stale
Copy link

stale bot commented Jan 3, 2022

This issue has been automatically marked as stale because of no recent activity. It will be closed in 10 weeks if no further activity occurs. Thank you for your contributions.

@alxzak alxzak moved this to Backlog in Kirby Feb 7, 2022
@MadsBuchmann
Copy link
Contributor

Is this worth taking a look at?
https://ionicframework.com/docs/api/title#collapsible-large-titles

@MadsBuchmann MadsBuchmann moved this from 📙 Backlog to 💡 Shaping in Kirby Feb 14, 2022
@bommariusser
Copy link
Contributor Author

how does that work @MadsBuchmann ? :D can you demo that?

@MadsBuchmann
Copy link
Contributor

@bommariusser got nothing to demo. I commented the above link as we stumbled upon it when discussing this issue yesterday. It's might just be a lever exposed by the ionic framework that we might use to implement the functionality, in a way exposed by the framework :D

@bommariusser
Copy link
Contributor Author

bommariusser commented Feb 15, 2022

alright @MadsBuchmann :) sounds great! The showcase is not so "not developer" friendly though so its hard to really understand what collapsible means :D sounds like something that gets smaller :P so like a sticky page-title kinda thing or font scaling kinda thing :)

@MadsBuchmann
Copy link
Contributor

MadsBuchmann commented Feb 24, 2022

Tech refinement

I've spent some time looking into three different solutions for this:

  1. Using ionics collapsible title functionality a PR with a POC has been uploaded in POC: Ionic collapsible title solution #2067.
  2. Simply making the title part of the content and never have it in the toolbar.
  3. Always having the title in ion-toolbar and truncating it on scroll

I think we should go with no. 1. I've written down some pros & cons for this solution in #2067. The second will not be implemented due to UX considerations. When researching the third solution i stumbled into the problems mentioned by @flemcito by not being able to control overflow behaviour.

There's still a couple of things to consider:

  • How do we make this so it becomes opt-in?
  • Is there a way to dynamically applying the solution? Such that if the title is too long for the modal it will automatically use the collapsible title functionality?
  • What should the API for using it be?
  • How to make this so it does not affect other types of modal than the default one.
  • The solution requires us not to use ion-title as it will truncate the title in ion-content - instead we should use a heading tag. Which level should this be?
  • Should we carry out [Housekeeping] Remove deprecated ModalConfig title #2060 as part of this? (i vote yes)
  • How do we ensure the title is not displayed on load (this is a more technical detail and probably does not have to be discussed in plenum. Just something to watch out for)
  • How do we make it possible to modify text-position as a consumer? In some cases it should be centered others left aligned.

After talking with @henrikvoetmand the styling for the title when it is part of the content should be the same as the title for page in #2030

@MadsBuchmann MadsBuchmann moved this from 💡 Shaping to 📙 Backlog in Kirby Feb 28, 2022
@RasmusKjeldgaard RasmusKjeldgaard moved this from 📙 Backlog to 🚀 In Progress in Kirby Mar 4, 2022
@MadsBuchmann MadsBuchmann moved this from 🚀 In Progress to 💤 Impeeded in Kirby Mar 23, 2022
@MadsBuchmann MadsBuchmann moved this from 💤 Impeeded to 🚀 In Progress in Kirby Mar 31, 2022
@MadsBuchmann MadsBuchmann moved this from 🚀 In Progress to 🔎 Review pending in Kirby Apr 4, 2022
@alxzak alxzak moved this from 🔎 Review pending to 👀 Review in progess in Kirby Apr 5, 2022
@jkaltoft jkaltoft moved this from 👀 Review in progess to 🚀 In Progress in Kirby Apr 22, 2022
@jkaltoft jkaltoft changed the title [Enhancement] Modal title should support long titles f Apr 25, 2022
@jkaltoft jkaltoft changed the title f [Enhancement] Modal title should support long titles Apr 25, 2022
@MadsBuchmann MadsBuchmann moved this from 🚀 In Progress to 👀 Review in progess in Kirby Apr 26, 2022
@MadsBuchmann
Copy link
Contributor

The gift that keeps on giving 😅... Status right now is that #2174 contains all necessary functionality for enabling collapsible titles for modals that are not routed.

The modal configs for routed modals are hard coded and this is giving us some API troubles... A seperate issue has been created for this: #2192

Repository owner moved this from 👀 Review in progess to ✅ Done in Kirby Apr 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project