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

Added a details page to the properties dialog #1962

Merged
merged 76 commits into from Oct 7, 2020
Merged

Added a details page to the properties dialog #1962

merged 76 commits into from Oct 7, 2020

Conversation

winston-de
Copy link
Contributor

@winston-de winston-de commented Sep 22, 2020

This pull request adds a details page to the properties window.
Screencaps:
image
image

@yaira2
Copy link
Member

yaira2 commented Sep 22, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yaira2
Copy link
Member

yaira2 commented Sep 22, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yaira2 yaira2 self-requested a review September 22, 2020 19:59
@ghost ghost added the needs-review label Sep 22, 2020
@yaira2
Copy link
Member

yaira2 commented Sep 22, 2020

/azp run

@mdtauk
Copy link

mdtauk commented Oct 2, 2020

image

The design for the Properties Dialog is pretty closely mirroring the existing Win32 one.

Did you consider re-thinking what the design should be like? Or did you just want to make it familiar and recognisable?

@winston-de
Copy link
Contributor Author

@tsvietOK

Example of different elements inside DataGrid

Yeah, but you'd end up with a giant list defining all the properties, like this:

image

which I didn't think would be much better than doing it in the XAML file. If it's better to do it programmatically, I can.

@mdtauk

The design for the Properties Dialog is pretty closely mirroring the existing Win32 one.
Did you consider re-thinking what the design should be like? Or did you just want to make it familiar and recognisable?

I wasn't the one who designed the page you showed, so I wouldn't know. The only design change that was made (in this PR), is transparent backgrounds for text boxes when they aren't selected or hovered.

image

@tsvietOK
Copy link
Contributor

tsvietOK commented Oct 3, 2020

@winiston2212 Yes this list is quite big, but at least all properties are grouped in one place, instead of a lot of elements in both xaml and cs files. So yes, it is better to do it programmatically.
That's what i get for now:
image

**Edit
Here is my implementation: https://github.com/tsvietOK/files-uwp/tree/details_prop
Some functions are broken, but the main structure understandable.

@lukeblevins
Copy link
Contributor

@tsvietOK Good idea. Though, perhaps we can use a ListView instead of DataGrid as we're now trying to distance future experiences from the Toolkit DataGrid.

@lukeblevins
Copy link
Contributor

@winiston2212 I think it's not important at this point to use a List control whether ListView or DataGrid, as we can iterate here later on in another PR.

@lukeblevins
Copy link
Contributor

@winiston2212 Again, I really appreciate your work on this. 🥇 I plan to conduct my testing and code review on this PR tomorrow.

@winston-de
Copy link
Contributor Author

@duke7553 Alright, sounds good.

For the future, I'm working on a program to generate a .cs file with the list of properties and their information, making it easier to switch.
https://github.com/winiston2212/PropertiesListGenerator

@tsvietOK
Copy link
Contributor

tsvietOK commented Oct 4, 2020

Do we really need to display all available file properties? I think the main goal is creating ListView with different controls inside it with ability to edit properties. So if you create this control the right way, there will not a problem to add more file properties.

@winston-de
Copy link
Contributor Author

@tsvietOK Just the ones that the current File Explorer currently shows, no?

@tsvietOK
Copy link
Contributor

tsvietOK commented Oct 4, 2020

@winiston2212 I just want to say, that first of all we have to create a control, which will hold, for example, 5 properties(with different controls). So we can improve control design and avoid unnecessary actions (like creating your code generator). It is not hard to add all properties to the code.

@winston-de
Copy link
Contributor Author

@tsvietOK There are only like half a dozen different property types: String, Double, UInt32, Multivalue Strings, DateTime, the occasional enum (which are normally UInt32s), and Bytes. Wouldn't it be easier if separate controls and converters were created for each type, then the generator determined which ones to use based off of the type of property? The generator can also fetch the text to use for enums. Then all that would have to be done is correct any mistakes the generator made. A generator is how I did most of the XAML code.

@yaira2
Copy link
Member

yaira2 commented Oct 5, 2020

@mdtauk Right now the goal is to implement something functional with a touch of fluent. There was an issue about rethinking the properties dialog but I can't find it, if you have any ideas, feel free to start a new issue so everyone can chime in.

@mdtauk
Copy link

mdtauk commented Oct 5, 2020

@mdtauk Right now the goal is to implement something functional with a touch of fluent. There was an issue about rethinking the properties dialog but I can't find it, if you have any ideas, feel free to start a new issue so everyone can chime in.

Nothing especially comes to mind, I was just bringing up the question - as this issue was talking about the details, and that could spiral out of control depending on what metadata is populated.

@winston-de
Copy link
Contributor Author

Nothing especially comes to mind, I was just bringing up the question - as this issue was talking about the details, and that could spiral out of control depending on what metadata is populated.

@mdtauk If you think of anything, please tell me.

yaira2
yaira2 previously approved these changes Oct 7, 2020
@yaira2
Copy link
Member

yaira2 commented Oct 7, 2020

:shipit:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Display dimensions of an image in the properties dialog
5 participants