8000 [DT-2881] info alignment by andrewzamojc · Pull Request #2741 · temporalio/ui · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[DT-2881] info alignment #2741

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 19 commits into from
Jun 6, 2025
Merged

[DT-2881] info alignment #2741

merged 19 commits into from
Jun 6, 2025

Conversation

andrewzamojc
Copy link
Contributor
@andrewzamojc andrewzamojc commented May 28, 2025 •
edited
Loading

Description & motivation 💭

This PR aligns the details in the workflow header, so the information is visually associated. To accomplish this, the PR:

  • Uses CSS grid to align detail values in a stack next to the labels.
  • Adds a new component DetailList. Semantically it is a definition list and offers a similar structure.
    • This component captures the required CSS grid.
    • Includes a DetailListColumn to position details into columns on larger screens. It is a div under the dl which is allowed in HTML. On small screens the columns stack.
    • Moves link and text functionality from WorkflowDetail component to the more generic DetailsList, including tooltips and truncation.
    • Updates WorkflowDetails to use DetailList instead of WorkflowDetail. Tested turning on deployments column and everything seemed ok.
    • Does NOT delete WorkflowDetail since it is used in a couple other places that can be updated in the future: version-details.svelte and workflow-summary.svelte.
  • Adds a little extra vertical space between major elements in the workflow header, to get closer to the design and feel less cramped.
  • Changes clock icon to the text "Duration" as requested in the design.
  • Move SDK to last column, requested by Design
  • Removed mono spaced font style from labels, requested by Design
  • The design mentions heading sizes, but they seem like they're already updated and consistent.

Screenshots (if applicable) 📸

BEFORE

Screenshot 2025-06-04 at 11 15 14 AM

AFTER

Screenshot 2025-06-06 at 11 12 32 AM

TRUNCATION

Screenshot 2025-06-05 at 3 49 04 PM

STACKED

Screenshot 2025-06-06 at 11 12 50 AM

Design Considerations 🎨

  • spacing is easy to tweak if needed
  • due to existing truncation and now the css grid, if a future design requires multi-line values, this component would need a code change

Testing 🧪

I don't see the <SDKLogo> so it's hard to know if that'll work :S How can I test this?

How was this tested 👻

  • Manual testing
  • E2E tests added
  • Unit tests added
  • screen reader

Steps for others to test: 🚶🏽‍♂️🚶🏽‍♀️

  • Run the code locally or in preview
  • Make a workflow and go to its details page
  • Notice the details near the top are aligned nicely
  • squish the page and see it truncate and then stack

Checklists

Draft Checklist

Merge Checklist

Issue(s) closed

Docs

Any docs updates needed?

@vercel Vercel
Copy link
vercel bot commented May 28, 2025 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
holocene ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 6, 2025 8:06pm

@CLAassistant
Copy link
CLAassistant commented May 28, 2025 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@andrewzamojc andrewzamojc force-pushed the DT-2881-info-alignment branch from 7156619 to 7aae5c2 Compare June 2, 2025 20:23
@andrewzamojc andrewzamojc force-pushed the DT-2881-info-alignment branch from 7aae5c2 to 62f3df1 Compare June 3, 2025 19:04
@andrewzamojc andrewzamojc marked this pull request as ready for review June 4, 2025 15:30
@andrewzamojc andrewzamojc requested review from rossedfort and a team as code owners June 4, 2025 15:30
andrewzamojc
andrewzamojc commented Jun 4, 2025
View reviewed changes
andrewzamojc
andrewzamojc commented Jun 4, 2025
View reviewed changes
src/lib/components/detail-list/detail-list.svelte
aria-label={ariaLabel}
>
{@render children()}
</dl>
Copy link
Contributor Author
@andrewzamojc andrewzamojc Jun 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Annotation: In general, this grid works like so:

  • implicit grid, always adding two columns, one for label one for value
  • the label and value components are always set to column 1 and column 2
  • the column component is a subgrid in both directions, making the above work in columns
  • the rowCount prop is required due to constraints of the CSS subgrid, which does not tend to create implicit rows. In the future we could use new CSS selectors like "sibling index" to count the elements in CSS.

@Alex-Tideman
Copy link
Contributor
Screenshot 2025-06-04 at 11 05 17 AM

Think we may need some responsive love. We might need to make all the columns equal widths instead of growing since some of these values can be very long

@andrewzamojc andrewzamojc force-pushed the DT-2881-info-alignment branch from 9f4e843 to d7adbd5 Compare June 5, 2025 15:45
@andrewzamojc
Copy link
Contributor Author

@Alex-Tideman Thanks for the canary data to test out real values and see that sdk logo!

Alex-Tideman
Alex-Tideman reviewed Jun 5, 2025
View reviewed changes
@andrewzamojc andrewzamojc merged commit 48ac8fe into main Jun 6, 2025
16 checks passed
@andrewzamojc andrewzamojc deleted the DT-2881-info-alignment branch June 6, 2025 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Alex-Tideman Alex-Tideman Alex-Tideman approved these changes

@rossedfort rossedfort Awaiting requested review from rossedfort rossedfort is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@andrewzamojc @CLAassistant @Alex-Tideman
0