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

Entity layout and minor UI changes #832

Open
wants to merge 8 commits into
base: 3.x
Choose a base branch
from
Open

Conversation

paul121
Copy link
Member

@paul121 paul121 commented Apr 23, 2024

My original motivation for this was to improve the display of the "Add new comment" field on entities. Ultimately I think the best solution is to make each of our layout regions use the "gin-layer-wrapper" styles so they get a background color/shadow and are lifted on the page to match the existing comment styles. Then decreasing the size of the comment text headers (h2 is pretty big) makes things look nice.

Last, some minor changes to make other css in farm_ui_theme consistent.

@paul121
Copy link
Member Author

paul121 commented Apr 23, 2024

Some before/after examples...

Assets

asset-before
asset-after

Logs

log-before
log-after

Comment on lines +15 to 18
/* Hide layout regions if there is no content. */
.layout--twocol .layout__region:not(:has(div)){
display: none;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm finding some situation where this might not work.

Screenshot from 2024-04-23 12-53-30

Copy link
Member Author

Choose a reason for hiding this comment

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

This seemed to be caused by an empty <div> that seems like it would have displayed error/warning messages but it didn't render any and I haven't been able to reproduce. I was in-between switching branches and clearing cache so it's possible it was a fluke but I do wonder if this is an issue.

@paul121
Copy link
Member Author

paul121 commented Apr 25, 2024

I will try and get some more screenshots (mobile!) with more data on these pages. Most notably log quantities and files that are attached to the page.

@paul121
Copy link
Member Author

paul121 commented May 13, 2024

A couple more screenshots with more data... definitely a few things to clean up, but the mobile LGTM!

Anyone have thoughts on the below? @mstenta @Farmer-Eds-Shed @Fosten @symbioquine

  1. Images are dropped in the right dashboard region, sometimes above other fields (is_location, is_fixed, see asset example).
    • I think we should consider adding in the Images field label and try to make images appear last in this column.
    • OR should we move images to the bottom with files? Only display the first image above?
  2. Files are dropped in the bottom dashboard region above comments, inconsistent styling.
    • We should add a Files field label to make it clear what these are. Also need to make sure there is some margin/spacing between the Files + Comments.
    • Also, because the Files are rendered in another gin-layer-wrapper, this makes the comments look a little weird... hmm. We could make comments behave similarly but if other modules add fields to this bottom dashboard region they would have some inconsistency as well.
    • Maybe consider making the files render in a collapsed details?
  3. Consider moving more "meta" fields to the right dashboard region?
    • I first noticed this for the "Land Type" field (see asset example). It seems this might make more sense under the "Asset Type" field in the right region. BUT.... the Land Type edit form is in the main region, not the sidebar... I don't know, the right sidebar doesn't need to only be "meta" fields...
    • Maybe move "Flags", "Log Category" to the right ?
    • Move "Land Type" above "Flags"
Asset

more-data
more-data-2

Log

log-more-data
log-more-data-2

Log mobile

mobile-log
mobile-log-2

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

Successfully merging this pull request may close these issues.

None yet

1 participant