Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

File modification date shine through on scroll #37871

Closed
wants to merge 1 commit into from
Closed

File modification date shine through on scroll #37871

wants to merge 1 commit into from

Conversation

hamza221
Copy link
Contributor

@hamza221 hamza221 commented Apr 21, 2023 •
edited
Loading

Summary

Steps to reproduce:

  1. go to files
  2. make sure you have enough files to scroll
  3. scroll
  4. Look at the top right corner
    Before :

image

After :

image

## TODO
  • ...

Checklist

@hamza221 hamza221 added design Design, UI, UX, etc. 3. to review Waiting for reviews labels Apr 21, 2023
szaimen
szaimen requested changes Apr 21, 2023
View reviewed changes
Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

I fear this will reintroduce #33872

@hamza221
Copy link
Contributor Author

hamza221 commented Apr 21, 2023 •
edited
Loading

It's not the case as far as i see

Screen.Recording.2023-04-21.at.18.30.57.mov

szaimen
szaimen requested changes Apr 21, 2023
View reviewed changes
Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

See

2023-04-21-201810.mp4

@hamza221
Copy link
Contributor Author

It should be good now, It looks good on my end. Can you please give it a test

@hamza221
Copy link
Contributor Author

Screen.Recording.2023-04-23.at.17.29.29.mov

@ChristophWurst ChristophWurst requested review from a team, skjnldsv, Pytal and szaimen and removed request for ChristophWurst and a team April 24, 2023 09:16
szaimen
szaimen requested changes Apr 24, 2023
View reviewed changes
Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

Alternative, less intrusive approach in #37895

@hamza221 hamza221 marked this pull request as draft April 25, 2023 18:46
@hamza221 hamza221 force-pushed the bug/files-scroll branch 2 times, most recently from 53b62ec to 0cfdce5 Compare May 1, 2023 15:57
@hamza221 hamza221 changed the title expand actions.creatable to the whole width File modification date shine through on scroll May 1, 2023
@hamza221
Copy link
Contributor Author

hamza221 commented May 1, 2023 •
edited
Loading

This is the only work around I found without any side effects on other components. I'm open for other suggestions 🤨

@hamza221 hamza221 marked this pull request as ready for review May 1, 2023 19:18
szaimen
szaimen requested changes May 2, 2023
View reviewed changes
Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

This seems to break on the public share link page:
image

@hamza221 hamza221 requested a review from szaimen May 8, 2023 15:02
github-advanced-security[bot]
github-advanced-security bot found potential problems May 8, 2023
View reviewed changes
apps/files/templates/list.php
@@ -32,27 +35,27 @@
<h2><?php p($l->t('No entries found in this folder')); ?></h2>
<p></p>
</div>
<table class="files-filestable list-container <?php p($_['showgridview'] ? 'view-grid' : '') ?>" data-allow-public-upload="<?php p($_['publicUploadEnabled'])?>" data-preview-x="250" data-preview-y="250">
<table class="files-filestable list-container <?php p($_['showgridview'] ? 'view-grid' : '') ?>" data-allow-public-upload="<?php p($_['publicUploadEnabled']) ?>" data-preview-x="250" data-preview-y="250">

Check notice

Code scanning / Psalm

PossiblyUndefinedArrayOffset

Possibly undefined array key $_['showgridview'] on ArrayAccess|array{dirToken: mixed, isPublic?: mixed, showgridview?: mixed, uploadMaxHumanFilesize?: mixed, ...<array-key, mixed>}
szaimen
szaimen approved these changes May 10, 2023
View reviewed changes
Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

Works now

@hamza221
expand actions.creatable to the whole width
2d694ea 
Signed-off-by: hamza221 <hamzamahjoubi221@gmail.com>
@hamza221
Copy link
Contributor Author

Squashed and rebased

@szaimen szaimen requested a review from a team May 30, 2023 14:14
@HannesJo0139
Copy link

So is anything happening here? Just updated 2 major versions to current 27.1.1 and it is still broken?

@hamza221
Copy link
Contributor Author

hamza221 commented Oct 7, 2023

the bug fixed on 28 with #39808 I think, But still exists on stable versions, should I change base then backport it once merged ? what do you think @szaimen

@szaimen
Copy link
Contributor

szaimen commented Oct 9, 2023

the bug fixed on 28 with #39808 I think, But still exists on stable versions, should I change base then backport it once merged ? what do you think @szaimen

yes, I guess so. However the approach seems to be a bit scary to me...

@solracsf solracsf added this to the Nextcloud 28 milestone Nov 21, 2023
@blizzz blizzz mentioned this pull request Nov 22, 2023
Merged
5 tasks
@blizzz blizzz modified the milestones: Nextcloud 28, Nextcloud 29 Nov 23, 2023
This was referenced Mar 12, 2024
Merged
Merged
Merged
@Altahrim Altahrim mentioned this pull request Mar 20, 2024
Merged
@hamza221 hamza221 marked this pull request as draft March 20, 2024 17:33
@skjnldsv skjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Jul 27, 2024
@skjnldsv skjnldsv closed this Aug 14, 2024
@skjnldsv skjnldsv deleted the bug/files-scroll branch August 30, 2024 07:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@szaimen szaimen szaimen approved these changes

@skjnldsv skjnldsv Awaiting requested review from skjnldsv skjnldsv was automatically assigned from nextcloud/server-frontend

@Pytal Pytal Awaiting requested review from Pytal Pytal was automatically assigned from nextcloud/server-frontend

Assignees
No one assigned
Labels
2. developing Work in progress design Design, UI, UX, etc.
Projects
None yet
Milestone
Nextcloud 29.0.5
Development

Successfully merging this pull request may close these issues.
8 participants
@hamza221 @HannesJo0139 @szaimen @AndyScherzinger @blizzz @Altahrim @solracsf @skjnldsv