Skip to content
This repository has been archived by the owner on Feb 13, 2020. It is now read-only.

Restyled & rearranged work show page to improve alignment, usability.… #153

Merged
merged 1 commit into from
Jun 22, 2018

Conversation

seanaery
Copy link
Contributor

… Partially addresses RDR-270.

@seanaery
Copy link
Contributor Author

Screenshots of these revisions in action on Work show, e.g., for an anonymous user vs. an admin.

work-show-admin
work-show-anon

Copy link
Contributor

@coblej coblej left a comment

Choose a reason for hiding this comment

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

@seanaery I'm probably not the best person to review the CSS so maybe @mikedaul can have a look at that. I do have one question/observation based on the screenshot. I noticed that the column break in the metadata block split between an attribute heading and its value. If there's a reasonable way to do so, I think it might look better if the column break occurred between attributes.

@seanaery
Copy link
Contributor Author

@coblej Great observation -- thanks. The column break issue is compounded by some limitations on how definition elements (<dl>, <dt>, <dd>) can be grouped (example debate by the WHATWG). I think I can probably fix it, but as a future revision that may include overriding some core Hyrax code to wrap the term-definition pairs in an element that can be styled to avoid breaking in the column-split. Before investing the time to research & do that I thought it could be useful to see column-splitting in action for live examples: regardless of the odd split points the team could decide that columnizing is undesired.

@mikedaul
Copy link
Contributor

I'm struggling to come up with an elegant way to keep the dt and dd from breaking apart across the flex columns. Maybe try wrapping each pair in a div and use 'break-inside: avoid' or something like that?

@seanaery
Copy link
Contributor Author

@coblej , @mikedaul -- OK, fixed the column break by wrapping each dt+dd group in a <div> per @mikedaul 's idea. I did the (admittedly repetitive) <div> wrap in the attribute_rows partial rather than incurring the cost of duplicating/overriding Hyrax's attribute_renderer in RDR.

@seanaery seanaery requested a review from coblej June 21, 2018 19:43
Copy link
Contributor

@coblej coblej left a comment

Choose a reason for hiding this comment

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

@seanaery I'm happy to approve this so it can be merged, though I admit the CSS is a bit over my head.

@seanaery seanaery merged commit 11b1b60 into release-0.2 Jun 22, 2018
@seanaery seanaery deleted the RDR-270-work-show-cleanup branch August 3, 2018 14:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants