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.

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

Feature: Added dimensions to tooltip when hovering over image files #15543

Merged
merged 8 commits into from
Jun 10, 2024

Conversation

0x5bfa
Copy link
Member

@0x5bfa 0x5bfa commented Jun 3, 2024

Resolved / Related Issues

To prevent extra work, all changes to the Files codebase must link to an approved issue marked as Ready to build. Please insert the issue number following the hashtag with the issue number that this Pull Request resolves.

Steps used to test these changes

Stability is a top priority for Files and all changes are required to go through testing before being merged into the repo. Please include a list of steps that you used to test this PR.

  1. Open Files app
  2. Hover over an image file
  3. See if it is corrupt image or dimention property is unavailable it wont be shown

Copy link
Member

@hishitetsu hishitetsu left a comment

Choose a reason for hiding this comment

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

The condition of dimensions display availability can be improved, but it works fine.

src/Files.App/Data/Items/ListedItem.cs Outdated Show resolved Hide resolved
src/Files.App/Data/Items/ListedItem.cs Outdated Show resolved Hide resolved
0x5bfa and others added 2 commits June 4, 2024 19:45
Co-authored-by: hishitetsu <[email protected]>
Co-authored-by: hishitetsu <[email protected]>
Copy link
Member

@hishitetsu hishitetsu left a comment

Choose a reason for hiding this comment

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

LGTM

@0x5bfa
Copy link
Member Author

0x5bfa commented Jun 7, 2024

Works fine on my end.
Ready for review again.

Copy link
Member

@yaira2 yaira2 left a comment

Choose a reason for hiding this comment

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

LGTM

@yaira2 yaira2 added ready to merge Pull requests that are approved and ready to merge and removed needs - code review labels Jun 10, 2024
@yaira2 yaira2 requested a review from hishitetsu June 10, 2024 20:01
@yaira2
Copy link
Member

yaira2 commented Jun 10, 2024

I noticed that it's not returning values for webp files, it would be nice to resolve this but it's not a blocker.

@yaira2 yaira2 changed the title Feature: Added image dimentions in hover-over summary Feature: Added dimensions to tooltip when hovering over image files Jun 10, 2024
@yaira2 yaira2 merged commit 31d6fee into files-community:main Jun 10, 2024
6 checks passed
@0x5bfa 0x5bfa deleted the 5bfa/Feature-AddDimentionsSummary branch June 12, 2024 04:24
@@ -326,6 +329,41 @@ public ObservableCollection<FileProperty> ItemProperties
set => SetProperty(ref itemProperties, value);
}

public string DimensionsDisplay
Copy link
Member

@yaira2 yaira2 Jun 26, 2024

Choose a reason for hiding this comment

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

I discovered this is fetching as soon as the location is opened, it should only be fetched when hovered for the first time.

Copy link
Member

Choose a reason for hiding this comment

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

This also causes #15202

Copy link
Member

Choose a reason for hiding this comment

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

I'll be commenting this out because the feature is low priority, but it'll be nice to add back once these issues are resolved.

Copy link
Member Author

Choose a reason for hiding this comment

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

@hishitetsu The first implementer said this way is the fastest however speaking of this should we use WinRT API to retrieve the dimentions of images?

Copy link
Member

Choose a reason for hiding this comment

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

I think the tooltip value can be empty at first, since the tooltip value is updated each time the pointer is entered.

private void Grid_PointerEntered(object sender, PointerRoutedEventArgs e)
{
if (sender is FrameworkElement element && element.DataContext is ListedItem item)
// Reassign values to update date display
ToolTipService.SetToolTip(element, item.ItemTooltipText);
}

@hishitetsu The first implementer said this way is the fastest however speaking of this should we use WinRT API to retrieve the dimentions of images?

I don't know if using the WinRT API will solve any problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Pull requests that are approved and ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: Show image resolution on hover information
4 participants