Skip to content

[fix] Finishing touches for next/previous without focus#2140

Merged
Alkarex merged 7 commits intoFreshRSS:devfrom
Frenzie:next-previous
Nov 18, 2018
Merged

[fix] Finishing touches for next/previous without focus#2140
Alkarex merged 7 commits intoFreshRSS:devfrom
Frenzie:next-previous

Conversation

@Frenzie
Copy link
Member

@Frenzie Frenzie commented Nov 18, 2018

Cf. #1767.

@Alkarex Alkarex added this to the 1.13.0 milestone Nov 18, 2018
@Alkarex Alkarex added the UI 🎨 User Interfaces label Nov 18, 2018

if (old_active[0] !== new_active[0]) {
if (isCollapsed && !skipping) { // BUG?: isCollapsed can only ever be true
if (isCollapsed && !skipping) { // BUG?: isCollapsed can only ever be true
Copy link
Member

Choose a reason for hiding this comment

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

The function declaration a bit higher up contains a default parameter, which is ES6.

function toggleContent(new_active, old_active, skipping = false) { }

This might not be compatible with all the browsers we support so far (e.g. IE11?)
I suggest removing the default parameter, and being explicit in all the calls

@Alkarex Alkarex merged commit e20c7ef into FreshRSS:dev Nov 18, 2018
@Frenzie Frenzie deleted the next-previous branch November 18, 2018 18:39
@Alkarex
Copy link
Member

Alkarex commented Nov 20, 2018

@Frenzie Now that you have become sed master of i18n files, could you please add the TODOs for the new strings? 😛 conf.shortcut.skip_next_article and conf.shortcut.skip_previous_article are visibly missing

@Frenzie
Copy link
Member Author

Frenzie commented Nov 20, 2018

Sure, but you might have to remind me in a few days. I'm heading off to bed soon.

Frenzie added a commit to Frenzie/FreshRSS that referenced this pull request Nov 28, 2018
Alkarex pushed a commit that referenced this pull request Nov 28, 2018
mdemoss pushed a commit to mdemoss/FreshRSS that referenced this pull request Mar 25, 2021
* [fix] Finishing touches for next/previous without focus

Cf. FreshRSS#1767.

* Avoid single quote

Alternative: use `’`

* Minor whitespace

* Minor whitespace

* be explicit about skipping

* add todos

* overshot by one
mdemoss pushed a commit to mdemoss/FreshRSS that referenced this pull request Mar 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

UI 🎨 User Interfaces

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants