Semantic-ui: SCROLLING not working with modal

[batata004]

Open this link below in Chrome:

http://jsfiddle.net/6oony3xu/4/

After 2 seconds the script appends content to the modal BUT the scrolling does not show up. Your screen has to be at last 768px tall (height) so you can reproduce this bug.

Scrolling should be automatically added if needed, I mean, if the content does not fit the window the scroll bars should appear as soon as content gets bigger than viewport.

This bug is really really really anoying cause it's very common that you want to display some that in the modal after it show up. If you have a form inside the modal and the user sends it with errors, you want to be able to show those errors inside the modal, thus, it gets taller but semantic ui does not add scrolling :(

[PhilLehmann]

Have a look here: http://jsfiddle.net/h76wmu47/.

You need to call modal('refresh') after changing your modals content in order for it to be recentered and possibly scrollbars to be attached/removed.

[batata004]

This is clearly a bug and I dont know why you refuse to fix it. Scroll bars should be needed automatically. Have you ever had to tell chrome to "refresh" everytime you append elements to the DOM? Have you ever had to tell Chrome to "refresh" everytime you append anything to small elements that use may or may not use scroll bars?

Scrollbars are universally configured as auto, they should auto enabl itself whenever it's needed.

[batata004]

And clearly there is a bug cause when the modal already starts with scroll bars, if I remove or add more content the modal itself add/remove scroll bar when nedded or not needed. This bug ONLY happens when the modal is initially shown without scroll bars, in this case it will never show scrollbar in the future if more content is added.

Please try to understand this bug: IF AT FIRST, when I cal modal("show"), THE MODAL IS ALREADY BEING SHOWN, IF I remove content the modal disappears. If I add content the modal reappers AUTOMATICALLY. This does not happen when the modal is shown initially without scrollbars, in this case this bug will happen everytime!!!

[PhilLehmann]

It's my understanding that the requirement to center modals conflicts with the requirement to automatically have scrollbars, hence the need for manual refresh.

If your requirements demand that modals are automatically receiving scrollbars, you could check the modals height in an interval or create a MutationObserver, depending on the nature of your DOM changes. The reason that such things are not implemented in the framework are surely the performance penalties inherent in such solutions. The most efficient solution is to manually call refresh as demonstrated in my fiddle after inserting the new content.

[batata004]

@philrykoff I agree with everything you said but your argument of the penalties is wrong (sorry for disagreeing). Did you read my last post? Did you see that this bug does not happen when the scroll bar is initially displayed?

I will explain again, please read:

SCENARIO 1 -> if you open the modal AND the scrollbars already are shown, you can remove content and they will AUTOMATICALY disappear. If you add content again the scrollbars will AUTOMATICALLY REAPPEAR!!!

SCENARIO 2 -> This does not happen if the scroll bars initially arent showing cause they will never show up again even if they are needed.

You see, if there is any penalty and your argument is right, it should not automatically display/hide scroll bars in SCENARIO 1, but it DOES. It works perfectly in SCENARIO 1, scrollbars are displayed/hidden ONLY WHEN NECESSARY!

[PhilLehmann]

If you like, I can explain the difference to you on gitter - query me over there.

[batata004]

What is gitter?

Anyway, is there someway I can make semantic ui ALWAYS display scroll bars when the modal is displayed? If so, this bug would not bother me anymore cause as I already said, when the scroll bars are shown initially they work as they should later when they are needed or not needed!

Sorry @philrykoff despite whatever you said this is clearly a bug, I think you should reopen it and allow another mod to reread what I said or ask @Banandrew to undo his labeling cause it's not correct.

[PhilLehmann]

I was actually wrong. Although this is not a good solution as stated before, it is included in the framework. Have a look at the observeChanges option.

[awgv]

@batata004 Gitter is one of our support resources. Please let me know If you’d like it to be reopened still; Jack will look at it, but it might take a while.

[batata004]

@Banandrew and @philrykoff thank you for being so supportive. I dont want to be rude, you are offering a great platform for devs all acrross the globe! I appreciate you attention into this and I would like to colaborate the best way I can.

I dont understand most of the things semantic ui does, I am not an advanced css dev... I am a good programmer (at least I think I am) at PHP, ASP, Java... but HTML, Javascript, CSS is not my "living". So the best way I can help is reporting 'bugs' hoping some of you may fix.

As a programmer of almost a decade I am pretty sure this behaviour that I reported in SCENARIO 2 is a bug cause in SCENARIO 1 it works exactly as it should. It's counter intuitive having SCENARIO 1 at the same time SCENARIO 2 does not work as expected just because it was initialized without scrollbars. I am 99% sure there will be no overhead supporting SCENARIO 2 cause SCENARIO 1 already implements it.

So, in my understanding this is a bug and should be fixed. It can take a day, a month, a year... I really dont care, I will use "refresh" as a workaround but if you want to build a solid platform it should behave the same in all situations, it should be intuitive.

Having said that, I believe this thread should be reopened and better evaluated. If a bug has an easy workaround (like using refresh every N seconds setInterval) it does not mean it's not a bug.

[PhilLehmann]

As said before, you can use the observeChanges option, which is exactly what you ask for (though it's more expensive than calling refresh manually). It automatically refreshes the position of the modal and it's scrollbars as soon as the DOM of the modal changes. If that does not work, please provide a fiddle.

[PhilLehmann]

Here is an updated fiddle demonstrating the use of observeChanges (without manually calling refresh): http://jsfiddle.net/5n8g9otx/.

[batata004]

A M A Z I N G! It worked using observeChanges! I am just thinking how it worked cause in the docs says:

observeChanges: Whether popup should automatically watch for its own removal from page to avoid memory leaks.

How is this behaviour helping to solve this problem?

[PhilLehmann]

It works because we are using modals, not popups. The docs say about the observeChangessetting of the modal module: Whether any change in modal DOM should automatically refresh cached positions.

[awgv]

@batata004 Yes, that’s actually Whether any change in modal DOM should automatically refresh cached positions. observeChanges uses MutationObserver under the hood that calls dropdown('refresh') when it detects any changes in the DOM.

[batata004]

@philrykoff now I understood and this setting is really usefull. But this option just explicits more the bug: in the docs says that the default to observeChanges is false. Ok, it's false, so when content is added to the modal it should never show scrollbars if it was not showing scrollbar. But this is wrong CAUSE when you open the modal already with scrollbars and you remove content from the modal then the scrollbars will disappears and if you put content back in the modal the scroll bars will reappears! If observeChanges is set to false it should never disappears and even worse, reappears.

This behaviour is not consistent with what one should expect from observeChanges. I think the first thing the community should change is set the default of observeChanges to TRUE (cause it's much much much more useful that way and avoid stupid people like me coming to foruns to complain about their modal dont showing scrollbars when content is added). AND AFTER setting the default to TRUE I think this bug that I reported here should be fixed, dont you agree?

[PhilLehmann]

As said before, there is no bug.

observeChanges is set to false as a sensible default, because the mechanism behind MutationObserver has some performance penalty. Because not everyone needs it, and not everyone want's to have that performance penalty, it's disabled by default.

The modal module has two well defined positioning algorithms:

a) center the modal
b) put the modal at the top of the page and show scrollbars if the content is too long to show on page (the well known overflow: auto behaviour)

Which behaviour is chosen, depends on whether the content is larger than the screen has space when the modal gets positioned.

The modal gets positioned when

1) it initially shows,
2) when the page is resized
3) when you manually call refresh
4) when the DOM of the modal changes (only if observeChanges is set to true)

So, when you open the modal and there is enough space on the screen, it is opened in mode (a), which you can see, because it is centered. If you then append content and none of the conditions in 1 - 4 are fulfilled, it is still centered and the bottom of the modal is not visible (off screen, because it is too long). Bad.

When you open the modal initially while it is too long to show on the screen, it is opened in mode (b). It is not centered, it is positioned at the top of the screen. Scrollbars are visible. If you then remove content and none of the mentioned conditions are fulfilled, it is still positioned at the top and the scrollbars will be hidden, because overflow: auto is set and the browser recognizes that it doesn't need scrollbars. The modal is not centered, as it's not in mode (a). Also bad. Due to overflow: auto, though, the scrollbars would resurface if you would again add content.

If you call refresh on the modal while in mode (b) and after removing content, mode (a) would be set and the modal would be nicely centered on the screen. If you had enabled observeChanges, the browser would have recognized that you changed the DOM (because it would have monitored it => shouldn't be done on too many DOM nodes, due to performance impact) and changed to mode (a) and center automatically.

I hope this explains why there is no bug and the current default settings make sense. I also hope I have not written any bogus :wink:

[batata004]

@philrykoff great explanation and it's very precise! Your explanation says exactly what is happening now, you described exactly what is the current behaviour!

You were very detailed and I appreciate your attention to explain all the use cases where the scrollbars would/wouldnt show up. I agree with everything you said, 100%.

Please dont give up on me, read this till the end, I think we will come to an agreement at some point.

My point here is that this behaviour is faulty, to say the least! You described what exactly happens with the current implementation.

@philrykoff recpectly, didnt you think it was too complicated to describe what is going on? Wouldnt it be easier to use the description below? It is simpler, easier to understand and much much more intuitive cause people will most of the time believe that the description below should be the default.

The description below is also what happens with JQUERY UI! I use a lot JQUERY UI and I am thinking about migrating other major websites to semantic UI, I just still didnt migrate cause semantic ui has still many rought corners that need to be corrected.

DESCRIPTION WHAT SHOULD HAPPEN, in my opinion, WHEN THE USER HAS MODAL IN HIS CODE:

The modal module has two well defined positioning algorithms:

a) center the modal
b) put the modal at the top of the page and _SET THE SCROLLBARS TO AUTO_

It does not matter if content is larger than the screen or is smaller, overflow:auto will have your back - always.

The modal gets positioned when

1. it initially shows,
2. when the page is resized
3. when you manually call refresh

You dont need to use any "DOM WATCH" manipulation, _no penaulty argument_! Modal will always use overflow:auto by deafult, so semantic-ui will always make sure your content appears in the screen, no matter what happens!

If, in any case, you want your content to be larger than the screen and you dont want it to be visible to users, _you are probably an idiot for intentionally hiding things from the user_ and if you request that feature you can open an ISSUE at GITHUB and we will mark it as an ENHANCEMENT - that will never gets done cause we have many other priorities to fix.

No need to call refresh, no need to attach events to DOM, no penaulty. Semantic-UI will always make sure overflow:auto is in place so you never need to spend hours and hours trying to convince other people that your solution is easier.

JQUERY UI is way more used than SEMANTIC UI, they have traveled a looong way, way before SEMANTIC UI. And with JQUERY UI the content is always visible if you use a modal, always. You can update the content, remove... the scrollsbars are always updated immediatly.

IT'S NOT INTUITIVE TO DEMAND USERS TO SET AN OPTION TO TRUE IF THEY WANT CONTENT TO BE ALWAYS VISIBLE!

IT'S NOT INTITUIVE NOR SMART CREATING A SETTING THAT DEFAULTS TO FALSE AND MAKE USERS HAVE THE SAME STUPID PROBLEM THAT I HAVE.

JQUERY UI has proven to be a golden standard. They have maany design flaws, their design conception is terrible... they are not great, that's why I am migrating some small websites I work for to SEMANTIC UI. But we need to make this library more intituive, starting by the docs that are not clear, examples frequently lack JS code (like dropdown, checkbox...)

[PhilLehmann]

As said before, setting the page to overflow:auto is incompatible with centering, so there either needs to be a MutationObserver or refresh to be called manually.

At least this is my opinion. Maybe someone else can chime in 😄

Otherwise, feel free to create a PoC using your fiddle...

[batata004]

@philrykoff JQUERY UI does that brilliantly without any "dom watch" event/manipulation. You dont need to take my word, you can open this link https://www.sitepor500.com.br/?aceitar_contrato in a big screen where no scroll bars will show up when you open it, and then click the ENVIAR button at the bottom. You will see that te error message will expand and the scrollbars will appears smoothly.

If you inspect the DOM object you will see there is no mutation observer at all: just plain and simple overflow:auto in the parent element of the modal - as it should be and as one should expect from a solid UI framework. See image below:

[jlukic]

Centering in SUI modals is done using negative margins. This has obvious drawbacks that whenever height changes, those negative margins need to be recalculated.

The library could have done this using tranform: translateY(-50%); but this would have affected animations that use transform so we opted out of using this approach.

This is generally why a lot of other modals in UI frameworks position themselves as relative to the top of the page, and dont use animations with transform, to allow for simpler code.

We probably will support both cases out of the box (modal at top, and modal centered) in the near future so that people dont have to worry so much about content sizing changing modal height.

I also think it's reasonable to include scrolling content as an option with overflow-y.

You can do this now yourself by adding a rule to your modal.overrrides

.ui.modal > .scrolling.content {
  overflow: auto;
  max-height: 500px; /* or some arbitary height */
}

I really want to get people used to the idea of extending the library as necessary to fit their use case.

[PhilLehmann]

As I said, the Semantic UI modal centering seems not to be compatible with overflow:auto on the page.

JQuery UI doesn't do centering, so it has not problem with overflow. You can see, that if you scroll your site, the modal scrolls with it (it does not stay centered). When the text is being added, the modal is not being re-centered (what the MutationObserver would do).

So it seems JQuery UI has other priorities. I don't know if centering is possible in JQuery UI at all, but in SUI you can do both when configuring the right settings.

[batata004]

Yeap, jQuery accepts centering and I always use it -> $("#janela").dialog('option', 'position', 'center');

I understand the difficulties of what you say, you made a very clear point and I totally agree with you! :) But I think that if you cant make use of native solutions as overflow:auto you shouldnt be in this business! Semantic ui is AWESOME and it is doing a really great job, and I think to keep up with the competitors it should copy the competitor's approach instead of following a longer, buggy and more complicated path!

I think you shouldnt use negative margins, I used to use that approach when I was a "script kid" where I used margin-left:-50% and adding the half of the width. This works but has drawbacks.

I think the positioning of the modal should be exactly as jquery ui does, it works perfectly and everytime! You dont see people complaining about invisible content/buttons...

JQUERY UI approach is to calculate left/top coordinates and recalculate them as window resize. You agree with me that window resize is a very rare event to fire, and when it fires it will sure have some overhead to recalculate the new let/top coordinates, but that's it, no observers, nothing.

[batata004]

EDIT: if you think the current approach that semantic ui is using to modals is good you probably didnt take a peek at this https://github.com/Semantic-Org/Semantic-UI/issues/4196 and this https://github.com/Semantic-Org/Semantic-UI/issues/4094

Those are very serious bugs, easily reproducable that affects A LOT THE usability. But they are still not even close to be fixed and worse, one of them is marked as low priority (which I cant believe cause it affects most of the users that have android). I dont even need to say that one of those bug reports is gonna make birthday in a couple of months.

[awgv]

@jlukic Jack, it’s a different problem, and simply allowing the modal’s content to be scrolled is not enough; he wants modals to always be scrollable like a modal that doesn’t fit into the viewport, but at the same time, it should also be centered when it does.

MutationObserver is being used by JavaScript frameworks and is fairly efficient for the task, so I think that having observeChanges is at least good enough at this point, and this issue can be closed. Please let’s not get ourselves into an endless and rather pointless discussion—it’s already been going for far too long.

[PhilLehmann]

If it's efficient... should we consider setting observeChanges to true for the next major release (if SUI follows SemVer as this would be breaking)?

[batata004]

@philrykoff I think if you, the devs, dont want to deal recreating all the modal structure to make it like jquery UI, the least you could do is change observeChanges to true. Doing this you solve a LOT of problems. People will never more come to SO or Github complaning that the content of their modal does not fit the viewport.

If something good we can carry from this thread is that making the default the most intuitive behaviour is a win-win. If some person wants its modal window content to be buggy and have the risk of the content being hidden in the viewport, that person can easily set observeChanges to false.

If you agree with me and if you agree with @Banandrew AND @philrykoff that mutationobserver is efficient, why not enable it by default?

I still think you should rebuild the whole modal HTML to make it use overflow:auto natively, but if you dont want to go that way I think that preventing content from being hidden is the best aletrnativa and the only way to solve this problem is seting observeChanges to true by default.

[awgv]

I don’t think we should be setting it to true, because it’s a use case, and it’s a developer’s responsibility to read the documentation. SUI is a popular, mature framework, and there’s a lot of different people using it; if we change it to true, a lof of people may be running MutationObserver for no reason to do so—and it might add time to a page load, especially to complex pages.

I’d maybe elaborated the description of observeChanges in the documentation to make it somehow clearer.

[PhilLehmann]

Ok. So what do you think about an info or warning message on http://semantic-ui.com/modules/modal.html#/usage ?

[batata004]

@Banandrew you can easily check if MutationObserver is set while observeChanges is also seted, so you dont need to execute if.

I give up, I spent a lot of time convincing you and it does not look we are gonna get anywhere. If you choose to set observeChanges to true by default we can say we dont have a bug BUT if you dont choose to set observeChanges to true by default, then we certainly have a bug for any standard of definition of what a bug is.

I say that because when the dev leaves observeChanges at its default (false) and then open a modal that fits the viewport (no scrollbars will be visible) and if he adds content to the modal the scrollbars WILL SHOW UP! THEY SHOULD NOT SHOW UP since observeChanges is set to false and the scrollbars should remain hidden.

I already repeated myself 3 or 4 times into this, so if you want to keep a bug in the SUI and a bug in the documentation ok, we are not dealing with pros.

Even an intermediate programmer knows that if some setting (observeChanges) guarantees that there will be no refresh AND a refresh happens anyway, this is a bug. Plain and simple.

I am closing this guys, I dont have much more time to discuss this. If you want to reopen it or discuss in another thread ok, but I will certainly not contribute to this anymore so it's completely up to you now.

[vinayakkulkarni]

I don’t think we should be setting it to true, because it’s a use case, and it’s a developer’s responsibility to read the documentation.

Read the docs. Get all the answers. Simple as that.

[awgv]

@philrykoff That makes sense! I can’t think of anything specific to put there right now, but I’ll submit something next week.

@batata004 I think you’re just very confused about the topic and the technologies that are involved, I recommend to dive into the source and corresponding technologies to understand it all better. Also, it’s not professional, nor it is mature to be disrespectful to unknown people on the internet—especially in the environment like GitHub—please restrain yourself from doing that in the future.