Named Link Testing

Topic: Named Link Testing (Read 4813 times) previous topic - next topic

0 Members and 1 Guest are viewing this topic.

Named Link Testing

February 09, 2017, 08:37:15 am

I placed some test code on the server yesterday, well to test!

I've noticed that there are times when you click "new" or go to a linked liked / mentioned message, you don't end up at the right spot on the page, usually overshooting the proper message. I don't know if it was just me seeing that behavior or if others have seen that as well.

In the past this was somewhat caused by the editor in the quick reply grabbing focus and messing things up. But thinking about it some more it may be caused by the code blocks we use. After a page loads the JS kicks in and makes the code blocks collapse to fit the code or set to a fixed height if there is a large code block. This moves the page about but the named link in the url (that #new or #msg1234) has already been navigated to.

So the testing code is some JS that runs a scrollTo after (and if) the code collapse code has triggered. Probably the full fix would be to prevent the browser default action (not sure you can) then add a scrollTo event as some promise from the code collapse, right now It just adds an on load event if the code code is run.

Anyway if you notice anything improved, unchanged or worse let me know.

Re: Named Link Testing

Reply #1 – February 09, 2017, 08:49:08 am

I noticed it jump up, for example here. On the one hand it's a bit odd, but on the other hand the alternative is weird too.

I think the main question here is why the <pre>s are so enormous at first? With something like height:auto (optional) and max-height:something there should be no need for any weird JS trickery.

Code: [Select]

overflow: auto;
display: block;
height: auto;
max-height: 200px;

Btw, what's the relevant commit hash/GH link?

Re: Named Link Testing

Reply #2 – February 09, 2017, 09:01:09 am

No GH link for the test code, just something I quickly put together and put in place to test.

No idea on the JS TBH, maybe its not needed any longer but was when we first began work on 1.0?. We could try new CSS and see if it works or not without the need for the JS swap.

ETA: Looks like this may have been an old ie8 issue with max heights and overflow auto ... bet we can drop all of that now

Re: Named Link Testing

Reply #3 – February 09, 2017, 09:45:56 am

Changed the site CSS and turned off the JS code stuff ... so lets see how that works

Re: Named Link Testing

Reply #4 – February 09, 2017, 09:53:25 am

Oh yeah, one of IE8's fun bugs was something along the lines of max-height combined with overflow:auto causing a page to appear completely blank.

There's actually a CSS-only workaround, see http://stackoverflow.com/a/881597/2470572

Code: [Select]

/*
SUPER nasty IE8 hack to deal with this bug
*/
pre 
{
    max-height: none\9 
}

NB I'm not saying to stick that in. Let IE8 rot, lol

Anyway, love the new script-less experience. Much better.

Re: Named Link Testing

Reply #5 – February 09, 2017, 10:00:27 am

For 1.1 we finally don't have to worry about ie8 :happy dance:

One side effect that I see of CSS only fix (which seems to work well) is that the resize handle does not allow the code block to be greater than max-height. I think only @emanuele would complain about that though

If we want to be able to expand a larger block of code, then some JS may need to be attached to that handle.

Re: Named Link Testing

Reply #6 – February 09, 2017, 10:39:41 am

No, I complain about that too now that you mention it.

(Or more to the point, I think the default max-height is too small and should be doubled or maybe even tripled. I'm less interested in dragging that thing around as such.)

Where does the handle come from? Is it added by prettify.js?

Re: Named Link Testing

Reply #7 – February 09, 2017, 10:58:15 am

Quote from: Frenzie – February 09, 2017, 10:39:41 amWhere does the handle come from? Is it added by prettify.js?

Code: (css) [Select]

resize: vertical;

Re: Named Link Testing

Reply #8 – February 09, 2017, 11:23:44 am

Ah, thanks! I wasn't aware of that new property. I thought it was just a thing browsers (thankfully!) did these days, but I didn't know it was by implementing a new CSS property and changing the default stylesheet.^[1]

Yeah, that does drastically reduce the usefulness of the thing.

So load with max-height, CSS is much quicker than JS.

then on DOMContentLoaded go:
height = computedStyle height
max-height = auto

that'll prevent any bouncing.

¹Yeah, yeah, new as in '09/'10-ish I'm sure.

↵

Re: Named Link Testing

Reply #9 – February 09, 2017, 11:25:25 am

Um, that oddness above after the footnote was not typed by me. Input:

Code: [Select]

stylesheet.[footnote]Yeah, yeah, new as in '09/'10-ish I'm sure. :P[/footnote] 

Yeah, that does drastically reduce the usefulness of the thing.

output:

Code: [Select]

stylesheet.[1]>Yeah, that does drastically reduce the usefulness of the thing.

Re: Named Link Testing

Reply #10 – February 09, 2017, 12:02:08 pm

Quote from: Spuds – February 09, 2017, 10:00:27 amOne side effect that I see of CSS only fix (which seems to work well) is that the resize handle does not allow the code block to be greater than max-height. I think only @emanuele would complain about that though If we want to be able to expand a larger block of code, then some JS may need to be attached to that handle.

And... what about dropping the handle and add a button "expand" that shows the entire block instead?
I guess most of the times it's "show me everything or just the snippet", it's kind of unusual that halfway from the max-height and "everything" is very helpful (at least to me

Re: Named Link Testing

Reply #11 – February 09, 2017, 12:07:22 pm

Yup I was thinking along the same lines .. leave the css for the page load and proper named link locations. Then with JS do something like

Code: [Select]

function elk_codefix()
{
	$('.bbc_code').each(function()
	{
		var $this = $(this);
		if ($this.get(0).scrollHeight > $this.innerHeight()) {
			$this.css('height', $this.height());
			$this.css('max-height', 'none');
		}
		else {
			$this.css('resize', 'none');
		}
	});
}

Which should remove the max-height constraints on any code blocks that have a scroll bar and remove the resize handle on those that do not.

Could also do as you suggested which should be

Code: [Select]

function elk_codefix()
{
	$('.bbc_code').each(function()
	{
		var $this = $(this);
		$this.css('height', $this.height());
		$this.css('max-height', 'none');
	});
}

Either should prevent the page jumpiness and allow for the resize:vertical css to work as expected.

Re: Named Link Testing

Reply #12 – February 09, 2017, 12:11:32 pm

QuoteUm, that oddness above after the footnote was not typed by me. Input:

Not sure whats going on there either ... something else to look at.

QuoteAnd... what about dropping the handle and add a button "expand" that shows the entire block instead?

Thats feature creep

... Could do that as well and it make sense nods .... Although one could get surprised how much stuff can be placed in a code block

ETA: JS "fix" enabled on the site

Re: Named Link Testing

Reply #13 – February 09, 2017, 12:24:10 pm

Maybe it would be better to feature creep into the profile settings? For example, in my DnD add-on^[1] I've got this:

Code: [Select]

		if (!$user_info['is_guest'])
		{
			loadMemberData($user_info['id']);
			if (!empty($user_profile[$user_info['id']]['options']['cust_fontsi']))
			{
				$context['html_headers'] .= '<style>html {font-size:'.$user_profile[$context['user']['id']]['options']['cust_fontsi'].'}</style>';
			}
		}

¹Private, not to keep it secret but because it's too tailored to a few site-specific things.↵

Re: Named Link Testing

Reply #14 – February 09, 2017, 12:42:05 pm

Thats not a bad idea at all ... amazing amount of churn over the font size on a site (or as its perceived on ones device)