Skip to content

fix(files): highlight previous folder on history up - #53285

Merged
skjnldsv merged 2 commits into
masterfrom
fix/files-position-navigation
Jun 3, 2025
Merged

fix(files): highlight previous folder on history up#53285
skjnldsv merged 2 commits into
masterfrom
fix/files-position-navigation

Conversation

@skjnldsv

@skjnldsvskjnldsv commented Jun 3, 2025

Copy link
Copy Markdown
Member

@skjnldsvskjnldsv added this to the Nextcloud 32 milestone Jun 3, 2025
@skjnldsvskjnldsv self-assigned this Jun 3, 2025
@skjnldsv
skjnldsv requested a review from a team as a code ownerJune 3, 2025 11:33
@skjnldsv
skjnldsv requested review from susnux and removed request for a teamJune 3, 2025 11:33
@skjnldsvskjnldsv added the 3. to review Waiting for reviews label Jun 3, 2025
@skjnldsv
skjnldsv requested review from artonge and sorbaughJune 3, 2025 11:33
@skjnldsvskjnldsv added feature: files papercut Annoying recurring UX issue with possibly simple fix. labels Jun 3, 2025
@github-project-automationgithub-project-automationBot moved this to 🏗️ In progress in 📁 Files teamJun 3, 2025
@skjnldsv
skjnldsvforce-pushed the fix/files-position-navigation branch from a3873e7 to bcfbd42CompareJune 3, 2025 11:33
@skjnldsvskjnldsv changed the title fix(files): highlight previosu folder on history upfix(files): highlight previous folder on history upJun 3, 2025
susnux
susnux reviewed Jun 3, 2025
View reviewed changes

@susnuxsusnux left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How does this belong to the hot key service? Seems to be generic for all navigation?

@skjnldsv

skjnldsv commented Jun 3, 2025

Copy link
Copy Markdown
MemberAuthor

How does this belong to the hot key service? Seems to be generic for all navigation?

Right, it's kind of hijacking the same shortcut as alt+up.
Pressing the back button is also kind of a hot key (I have dedicated mouse and keyboard buttons for it) 🤷

@susnux

susnux commented Jun 3, 2025

Copy link
Copy Markdown
Contributor

Pressing the back button is also kind of a hot key (I have dedicated mouse and keyboard buttons for it) 🤷

Yes but this is basic routing / history handling, no? Would this not better suit the router directly?
I would not expect it in the hot key service to be honest.

@susnux

susnux commented Jun 3, 2025

Copy link
Copy Markdown
Contributor

(approved as the code itself is good, just the location 🤷 )

@skjnldsv

skjnldsv commented Jun 3, 2025

Copy link
Copy Markdown
MemberAuthor

Pressing the back button is also kind of a hot key (I have dedicated mouse and keyboard buttons for it) 🤷

Yes but this is basic routing / history handling, no? Would this not better suit the router directly? I would not expect it in the hot key service to be honest.

Fair to me!
let me move it :)

@skjnldsv
skjnldsvforce-pushed the fix/files-position-navigation branch 2 times, most recently from a382619 to 5b5d5dbCompareJune 3, 2025 15:08
@skjnldsv

skjnldsv commented Jun 3, 2025

Copy link
Copy Markdown
MemberAuthor

And with cypress tests

@skjnldsv

skjnldsv commented Jun 3, 2025

Copy link
Copy Markdown
MemberAuthor

/compile rebase

skjnldsvand others added 2 commits June 3, 2025 15:38
@skjnldsv@nextcloud-command
fix(files): highlight previous folder on history up
5974649
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@nextcloud-command
chore(assets): Recompile assets
9a30a8f
Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
@nextcloud-command
nextcloud-commandforce-pushed the fix/files-position-navigation branch from 5b5d5db to 9a30a8fCompareJune 3, 2025 15:40
@nextcloud-command
nextcloud-command requested a review from a team as a code ownerJune 3, 2025 15:40
@skjnldsv
skjnldsv enabled auto-merge June 3, 2025 15:53
@skjnldsv

skjnldsv commented Jun 3, 2025

Copy link
Copy Markdown
MemberAuthor

/backport 5974649 to stable31

@skjnldsv

skjnldsv commented Jun 3, 2025

Copy link
Copy Markdown
MemberAuthor

/backport 5974649 to stable30

susnux
susnux approved these changes Jun 3, 2025
View reviewed changes

@susnuxsusnux left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice 🚀

@skjnldsv
skjnldsv merged commit 97a9a68 into masterJun 3, 2025
@skjnldsv
skjnldsv deleted the fix/files-position-navigation branch June 3, 2025 16:36
This was referenced Jun 3, 2025
Closed
Merged
jancborchardt
jancborchardt reviewed Jun 4, 2025
View reviewed changes

@jancborchardtjancborchardt left a comment

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.

@skjnldsv thanks a bunch! :) Just to clarify, the issue #53031 was mostly about "Scroll position within folders is lost on navigation", is that also fixed with this pull request?

@skjnldsvskjnldsv moved this from 🏗️ In progress to ☑️ Done in 📁 Files teamJun 4, 2025
@skjnldsv

skjnldsv commented Jun 4, 2025

Copy link
Copy Markdown
MemberAuthor

@skjnldsv thanks a bunch! :) Just to clarify, the issue #53031 was mostly about "Scroll position within folders is lost on navigation", is that also fixed with this pull request?

Yes, should be :)

@jancborchardt

jancborchardt commented Jul 30, 2025•
edited
Loading

Copy link
Copy Markdown
Member

@skjnldsv is this deployed on daily yet? I just tried with both:

  • Going back via browser controls
  • Going up to parent folder via breadcrumbs

And both still end up at the top of the parent folder. Let me know if it’s not on daily yet. :)

@skjnldsv

skjnldsv commented Jul 30, 2025

Copy link
Copy Markdown
MemberAuthor

No clue what version daily is.
But it should be deployed on c.nc.com already (Nextcloud 31.0.6)

@susnux

susnux commented Jul 30, 2025

Copy link
Copy Markdown
Contributor

On c.nc.com both work (and also on current master branch)

@jancborchardt

jancborchardt commented Jul 31, 2025

Copy link
Copy Markdown
Member

Just tested again on c.nc, and while the previous folder is highlighted, the scroll position still ends up on the very top instead of keeping the position → as asked in the original issue, ref #53285 (review)

I also cleared cache and everything, not sure what issue it could be?

@skjnldsvskjnldsv mentioned this pull request Aug 19, 2025
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jancborchardtjancborchardtjancborchardt left review comments
@susnuxsusnuxsusnux approved these changes
@miaulalalamiaulalalamiaulalala approved these changes
@artongeartongeAwaiting requested review from artongeartonge is a code owner automatically assigned from nextcloud/server-frontend
@sorbaughsorbaughAwaiting requested review from sorbaughsorbaugh is a code owner automatically assigned from nextcloud/server-frontend

Assignees

@skjnldsvskjnldsv

Labels

3. to reviewWaiting for reviewsenhancementfeature: filespapercutAnnoying recurring UX issue with possibly simple fix.

Projects

📁 Files team
Status: ☑️ Done

Milestone

Nextcloud 32

Development

Successfully merging this pull request may close these issues.

Scroll position within folders is lost on navigation

5 participants

@skjnldsv@susnux@jancborchardt@miaulalala@nextcloud-command