Skip to content

Dummy space to enable full scroll to mark as read.#1974

Closed
primaeval wants to merge 0 commit intoFreshRSS:masterfrom
primaeval:master
Closed

Dummy space to enable full scroll to mark as read.#1974
primaeval wants to merge 0 commit intoFreshRSS:masterfrom
primaeval:master

Conversation

@primaeval
Copy link
Copy Markdown
Contributor

Hi. Here is my proposed solution to allow full Scrolling to Mark As Read in a similar way to Feedly.

It adds a non-breaking space to the end of the Mark All as Read button with a height of 100vh ie the height of the viewport.

I'm sure there is a more elegant way to do it but this works and is quite simple.

Copy link
Copy Markdown
Member

@aledeg aledeg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be something that you add in the css extension and shouldn't be in the core.

<span class="bigTick">✓</span><br />
<?php echo _t('gen.pagination.mark_all_read'); ?>
<?php echo _t('gen.pagination.mark_all_read'); ?><br />
<span class="bigSpace">&nbsp;</span>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not a big fan to add markup just for the sake of formatting. I think you should write a CSS rule to match what you want to do.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've succeeded while using the custom css extension configuration with the following rule:

#stream:after {content: "\00A0";font-size: 100vh;}

I am really not sure we should add that in core.

Anyway, thank you for your contribution.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks a bit cryptic but it is easier with a single line. Leaving it for a user to work that out is probably asking a bit much.

Something like making the bigMarkAsRead button bigger to 100vh would be better. It probably needs some flex magic but I only dabble in html. I couldn't work out how to do it without looking weird.

After following the upset that happened when Feedly broke the scroll to mark as read functionality I can assure you that there are a huge number of users that like and expect that behaviour in a modern feed reader. I'm sure they would appreciate it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I'm missing something, but this pretty much just looks like a really convoluted way of adding some padding at the bottom? (That remark applies to both the generated content and to adding it directly into the markup.)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tried to add a padding, but I was unsuccessful. If you come up with a different solution, be my guest.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks like a good idea.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm afraid I still fail to see the difference with a simple padding-bottom:100vh.

(Also, does display:block even work on generated content in Fx? That's annoyed me for like 15 years. Are you telling me they finally fixed it?)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Frenzie If you can make it work with a padding-bottom, please show me 'cause I can't :'(

And yes it works on generated content :)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, maybe I'm partially confused with generated content on images, but I'm pretty sure Fx didn't do display:block on generated content in the past either. Opera/Presto already did generated content and media queries correctly in '03; it took the others about a decade to mostly catch up (but not quite!).

// all the lame browsers, namely those other than Opera/Presto and Prince, don't
// do CSS generated content on images...

function show_img_titles(img) {
	img.after('<div class="show-img-title">Mouseover title: ' + img.attr('title') + '</div>');
}

function init_show_img_titles() {
	if (window.$) {
		$('img[title]').each(function(){
			show_img_titles($(this));
		});

		var observer = new MutationObserver(function(mutations) {  
			mutations.forEach(function(mutation) {
				for (var i = 0; i < mutation.addedNodes.length; i++) {
					//console.log(mutation.addedNodes[i])
					$(mutation.addedNodes[i]).find('img[title]').each(function(){
						show_img_titles($(this));
					});
				}
			});
		});
		observer.observe(document.querySelector('#stream'), { subtree: true, childList: true });
	}
	else {
		window.setTimeout(init_show_img_titles, 50);
	}
}

init_show_img_titles();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah, sorry.

If you can make it work with a padding-bottom, please show me 'cause I can't :'(

I'm not sure what problem you were having. Did you accidentally put the padding-bottom above the padding? As in the latest commit by @primaeval it looks exactly like how I meant it.

@aledeg aledeg added UI 🎨 User Interfaces Extension 🔌 labels Aug 9, 2018
@primaeval
Copy link
Copy Markdown
Contributor Author

primaeval commented Aug 10, 2018

@Frenzie adding padding-bottom 100vh does seem to be the simplest solution as in the last commit.

You could even split the padding to make the button still visible when the articles go off the top of the screen.

@Frenzie
Copy link
Copy Markdown
Member

Frenzie commented Aug 10, 2018

@primaeval Yes, I mainly prefer it that way because the intent will be immediately obvious when reading without trying to figure out what [insert random element or generated content] is doing there.

It'd have the additional advantage of working on pretty much every browser ever, except that ship pretty much sailed with the vh.

@primaeval
Copy link
Copy Markdown
Contributor Author

I'm not sure I understand your vh comment. Do you mean it is ok or are there still problems?

I just checked Feedly. Their "Mark all as read" button disappears off the top of the screen when you scroll. That is the same as this fix at the moment.

Is there any situation when this could fail, that you know of?

@Frenzie
Copy link
Copy Markdown
Member

Frenzie commented Aug 10, 2018

I don't think of the current behavior of a problem, and of falling back to it would be graceful degradation. I'm just saying that vh isn't supported until approximately 2013 in more mainstreamish browsers and still isn't in Netsurf afaik.

Is there any situation when this could fail, that you know of?

Fail meaning what exactly? :-)

@primaeval
Copy link
Copy Markdown
Contributor Author

2013 is ancient in internet years. ;) I think we should be ok. I suppose it could have a big pixel or em value as a fallback if really necessary.

By fail I meant any strange mode or setting of FreshRSS. I don't use much more than the default settings, except the scroll to mark as read functionality. I've only been using it for a few weeks but I'm getting hooked on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants