Repository navigation
feat(authz): tighten title layout spacing and shrink the header action buttons - #249
Conversation
|
Thanks for the pull request, @jacobo-dominguez-wgu! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
openedx-webhooks
added
open-source-contribution
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #249 +/- ##
==========================================
+ Coverage 98.53% 98.71% +0.17%
==========================================
Files 73 80 +7
Lines 1299 1396 +97
Branches 426 468 +42
==========================================
+ Hits 1280 1378 +98
+ Misses 19 18 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
It matches the Figma. However I have a question:
The PR description says:
tightens the header band spacing and action-button sizing for visual consistency
For me is not clear, what is the reference I need to validate to ensure the UI is consistent?
For example, reducing the spacing in the header is not consistent if I compare with other applications titles such as Account or Catalog.
7c1d507 to
dc32f1c
Compare
Thanks, I have applied your suggestions. The intention was to keep consistency with studio home button sizes, but I have updated the description to reduce noise. |
dcoa
left a comment
dcoa
left a comment
There was a problem hiding this comment.
I will leave my approve, so you can merge early. But please take in consideration the following nits
| size="sm" | ||
| iconBefore={Plus} | ||
| onClick={handleClick} | ||
| className="font-weight-light" |
There was a problem hiding this comment.
I think this was added when you updated the PR but this is not needed, the Figma shows font-weight: 400 that is the default of paragon button class
| className="font-weight-light" |
There was a problem hiding this comment.
Applied.
| <Row className="mt-4"> | ||
| <Col xs={12} md={7} className="mb-4"> | ||
| <div className="d-flex align-items-center flex-column-sm"> | ||
| <div className="page-band py-3 bg-light-100"> |
There was a problem hiding this comment.
I was just reviewing the Figma and noticed that this area has a height of 86px, but the current one gives a total of 72px so:
| <div className="page-band py-3 bg-light-100"> | |
| <div className="page-band py-4 bg-light-100"> |
It will result in 88px that is closer to the Figma value.
There was a problem hiding this comment.
Applied
dc32f1c to
a5ce29a
Compare
|
🎉 This PR is included in version 1.0.0-alpha.7 🎉 The release is available on: Your semantic-release bot 📦🚀 |
5 participants
Description
AuthZTitlepreviously rendered aBreadcrumbunconditionally, so pages with no navigation context (noactiveLabeland nonavLinks) still showed an empty, orphaned breadcrumb. This PR renders the breadcrumb only when there is something to show, and tightens the header band spacing and action-button sizing.Supporting information
Fixes #238
Figma https://www.figma.com/design/xnmQJq1cTRVqNrs31R8zDM/AuthZ---v2?node-id=40000174-2&p=f&t=AvQgCUER8u7BJBiE-0
Testing instructions
AuthZTitlewithout anactiveLabelornavLinks(e.g. the AuthZ home) and confirm no breadcrumb is shown.navLinksand/or anactiveLabel(e.g. the audit user page) and confirm the breadcrumb still renders with the correct links and active label.npx jest src/authz-module/components/AuthZTitle.test.tsx— all tests pass, includingdoes not render the breadcrumb when there is no activeLabel and no navLinks.Other information