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

fix: tup-615 inconsistent header spacing - left & vertical #339

Merged
merged 46 commits into from
Nov 14, 2023

Conversation

wesleyboar
Copy link
Member

@wesleyboar wesleyboar commented Oct 10, 2023

Overview

Make header spacing consistent.

Related

Changes

Testing

  1. Open all sections.
  2. Verify all header text has the same space before and above.
  3. Verify, in Dashboard section, on large window (or with small text):
    • no excess space above "Dashboard" header text
    • no excess space between "Active Projects" and "My Tickets" table

UI

dashboard proj alloc project
tickets system account
dashboard layout
dashboard - full

The cut-off System Status "Running Jobs" is caused by #312, not this.

@wesleyboar wesleyboar changed the title feat: tup-615 small header buttons (#338) fix: tup-615 inconsistent header spacing Oct 10, 2023
wesleyboar and others added 3 commits October 10, 2023 16:19
Designers do not intend nor like big gap. It's been around since Portal.

But it's been so difficult to fix… until  `project-ticket-grid` came.

I hadn't noticed the fix was possible until just now.
@wesleyboar wesleyboar marked this pull request as ready for review October 10, 2023 21:47
wesleyboar and others added 23 commits October 11, 2023 16:41
* feat: tup-619 c-content block migration

* removed content-block import
* migrated c-update to core-styles

* removed c-update import
* feat: tup-624: added generics/attributes to demo

* migrated rest of attributes.css to core-styles

* removed unnecessary comments in css file for c-button
* getting rid of django.cms.blog.app.item.css

* removing the import for django.cms.blog.app.item.css
WARNING: v2.17.3 causes login form field bug.
No need for u-highlight per jira ticket

Co-authored-by: Wesley B <[email protected]>
* I believe this has already been merged into core-styles... Padding and Margin seem to be available on prod

* remove import of core-styles o-heading-steps.css
* feat/tup-633: Migrate lightgallery to core-cms

* delete unnecessary import of django.cms.picture.css
@R-Tomas-Gonzalez R-Tomas-Gonzalez self-requested a review November 14, 2023 16:52
Copy link
Contributor

@R-Tomas-Gonzalez R-Tomas-Gonzalez left a comment

Choose a reason for hiding this comment

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

Question on headers and consistency. I noticed that some headers have nested headers, and some do not. The headers that are not nested, seem to have padding-right, due to being within articles. The headers that are nested, span the whole width of the page. Right padding may not matter. But thought I'd mention it.
Here are the examples:

Screenshot 2023-11-14 at 11 20 06 AM Screenshot 2023-11-14 at 11 19 10 AM Screenshot 2023-11-14 at 11 19 50 AM Screenshot 2023-11-14 at 11 19 33 AM

@wesleyboar
Copy link
Member Author

Hm, thanks @R-Tomas-Gonzalez for #339 (review).

I did not pay attention to the right side of the headers…

I'll check this, and how #335 interacts with it.

@wesleyboar
Copy link
Member Author

@R-Tomas-Gonzalez What do you think consistent space on the right being handled in a new ticket instead of this one?

My reasoning…
  • original ticket was focused on left and vertical
  • this PR seems to be a valid fix for left and vertical
  • i think it may be easier to tackle right-hand side in isolation
  • i think it will be easier to tackle right-hand side after merges of existing PRs

@R-Tomas-Gonzalez
Copy link
Contributor

@R-Tomas-Gonzalez What do you think consistent space on the right being handled in a new ticket instead of this one?

My reasoning…

  • original ticket was focused on left and vertical
  • this PR seems to be a valid fix for left and vertical
  • i think it may be easier to tackle right-hand side in isolation
  • i think it will be easier to tackle right-hand side after merges of existing PRs

I agree. 👍 :)

@R-Tomas-Gonzalez
Copy link
Contributor

Will approve when conflicts are resolved.

@wesleyboar wesleyboar changed the title fix: tup-615 inconsistent header spacing fix: tup-615 inconsistent header spacing - left & vertical Nov 14, 2023
@wesleyboar
Copy link
Member Author

wesleyboar commented Nov 14, 2023

Copy link
Collaborator

@jarosenb jarosenb left a comment

Choose a reason for hiding this comment

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

Approving now that long project lists no longer overflow the column.

@wesleyboar
Copy link
Member Author

wesleyboar commented Nov 14, 2023

Warning
Do not merge. Diff has accidental merge of CSS migration via 0e222ee.

@wesleyboar wesleyboar marked this pull request as draft November 14, 2023 22:45
…onsistency"

This reverts commit 0e222ee, reversing
changes made to d6215e3.
@wesleyboar
Copy link
Member Author

Important
Safe to merge. Problem resolved. I reverted 0e222ee via f6b7bd5. Retested. Re-reviewed diff.

@wesleyboar wesleyboar marked this pull request as ready for review November 14, 2023 22:56
This was fixed by #369, which is merged in here via main.

But #369 only worked because it was tested at zoom of maybe 90%.

When tested (in PR #339) at zoom 100%, cut off still happens.

So, I'm jsut gonna tweak ratio slightly to avoid cut off.
grid-template-columns: 0.65fr 0.35fr;
grid-template-columns: 0.6fr 0.4fr;
Copy link
Member Author

@wesleyboar wesleyboar Nov 14, 2023

Choose a reason for hiding this comment

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

This performs #369 again (see 16c2c0c), but is tested at 100% zoom.1

Footnotes

  1. Seems I had tested Fix/tup 598 running jobs cut off #369 at 90% zoom. 🫤

@wesleyboar wesleyboar merged commit 5b0a2b6 into main Nov 14, 2023
1 check passed
@wesleyboar wesleyboar deleted the fix/tup-615-header-spacing-consistency branch November 14, 2023 23:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants