-
-
Notifications
You must be signed in to change notification settings - Fork 780
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
Update scene, image, and gallery detail panels #4582
base: develop
Are you sure you want to change the base?
Conversation
I've briefly tested this build. I suspect much of my feedback may be regarding incomplete work, but I thought I'd write this out anyway. 1. CSS and LayoutThe layout displays detail items titles (aka the item heading) and detail item values using h5 and h6 tags respectively. This contributes to an inconsistent look with the other detail pages (performer, tag, studios) that we are trying to emulate. Why not just re-use the CSS and the structure those pages are using: <div class="detail-item {NAME}">
<span class="detail-item-title {NAME}">...</span>
<span class="detail-item-value {NAME}">...</span>
</div> Where <div class="detail-group">
<div class="detail-item studio">
<span class="detail-item-title studio">...</span>
<span class="detail-item-value studio">...</span>
</div>
<div class="detail-item release-date">
<span class="detail-item-title release-date">...</span>
<span class="detail-item-value release-date">...</span>
</div>
<div class="detail-item studio-code">
<span class="detail-item-title studio-code">...</span>
<span class="detail-item-value studio-code">...</span>
</div>
</div> Aside from the consistency, reliability, and prospective ease of maintenance by reusing the same CSS and structure, I suspect this will make it a lot easier to add the order property to above CSS. The order property is important because it allows the devs (and users through custom css) to easily change the order of detail items. detail-item.studio { order: 1; }
detail-item.release-date { order: 2; }
detail-item.studio-code { order: 3; }
detail-item.tags { order: 4; }
detail-item.updated-date { order: 5; }
detail-item.created-date { order: 6; }
detail-item.details { order: 7; }
detail-item.director { order: 8; }
... 2. What does the date heading mean?I believe we should take this opportunity to rename the date heading and its corresponding class name to 3. Detail items with null values should not be displayedWhen detail items have null values they're still being displayed. Per consistency with the detail pages we're imitating, it should not do this. 4. Created at and Updated at dates need stylingThose values still don't have the same styling as the other detail items 5. Details expand/collapse buttonI'm thinking it might be optimal to replace this button with |
Hard disagree here. Release date is one interpretation of the date field, but I don't think it's one that should be blanket applied. Users may have studio release, amateur videos or personal content, or a mix of all three and |
That's reasonable. I noticed after posting you also expressed concern for displaying a null field and the preference for a show more/show less functionality for details. Does this mean you're in agreement with the rest of my feedback, namely points 1 and 4. |
Yep, though my preference would be to use |
I agree with WP about this. The logos are more recognizable than text, so we shouldn't lose that completely.
I agree with all of this.
I actually like Q's approach to those fields. Those fields are really not the same in that they are not editable by the users. I don't think their style should be the same as the editable fields, and I agree with them being tucked away at the bottom. I'm not sure I like the rating being presented the same way as the other fields. It currently looks awkward to me and is not handled this way on the other details pages. I haven't thought of a better solution yet. |
This is an arbitrary or subjective reason to break the consistency of the paradigm. Resolution is not editable either; both resolution and creation date are immutable, the updated date will change as often as the user modifies the metadata making it more malleable than the latter in some fashion. But I do understand what you are communicating and I agree with the implication that different types of metadata should be visually distinct, that is, grouped together, and apart from another. To me it seems like the simplest, and universal method here, exemplified by guis like Windows File Properties modal and MD3 lists from their style guide, involves visually segmenting different types of metadata. They achieve it through horizontal line separators or header labels or both, to maintain uniformity across their presentation. Using this principle, this is how I've always presented the concept which this design is derived from. Where all the metadata types are presented cohesively in one tab, grouped by similarity and visually partitioned for clarity. However, this approach wasn't adopted by the author, either because it was deemed outside the scope of what the author wanted to implement, or the finds that principle undesirable. There is another thread somewhere, where I discuss the logical ordering of the metadata, and this might be considered beyond the scope of this specific commit. But I suspect neglecting a discussion of that broader perspective will lead to situations like this, where small parts of the UI are addressed without a comprehensive consensus or understanding of the ultimate end goal. 🤷
Maybe the scene creation and scene updated dates should be grouped with History. But I think that's for another debate once the paradigm/end goal of the presentation has been established.
For consistency with other pages, the only solution is to place the rating stars below the scene title. |
I feel the same way about the resolution. It shouldn't be presented on the tab. I've had CSS hiding it locally on my end for so long that I forgot it was ever a field on that tab. That information already lived in the files tab and was unnecessarily duplicated. Moving the created and updated date info to the file to a different tab crossed my mind, but I'm personally still happy with Q's approach. That information does not need to be front and center.
That's a good point. If we are going to utilize the space below the title, we may as well place the o-counter and organized button on that row as well, so the scene title doesn't need to be ellipsed. |
Show all details
setting to persist expanded details/description