Dummy space to enable full scroll to mark as read.#1974
Dummy space to enable full scroll to mark as read.#1974primaeval wants to merge 0 commit intoFreshRSS:masterfrom
Conversation
aledeg
left a comment
There was a problem hiding this comment.
I think this should be something that you add in the css extension and shouldn't be in the core.
app/views/helpers/pagination.phtml
Outdated
| <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"> </span> |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.)
There was a problem hiding this comment.
I've tried to add a padding, but I was unsuccessful. If you come up with a different solution, be my guest.
There was a problem hiding this comment.
That looks like a good idea.
There was a problem hiding this comment.
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?)
There was a problem hiding this comment.
@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 :)
There was a problem hiding this comment.
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();There was a problem hiding this comment.
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.
|
@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. |
|
@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 |
|
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? |
|
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
Fail meaning what exactly? :-) |
|
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. |
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.