-
Notifications
You must be signed in to change notification settings - Fork 16
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
Make all tabs visible for a workflow #1931
Conversation
Quality Gate passedKudos, no new issues were introduced! 0 New issues |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## release/2.12.0 #1931 +/- ##
==================================================
+ Coverage 40.41% 40.43% +0.01%
==================================================
Files 369 369
Lines 11493 11493
Branches 2948 2948
==================================================
+ Hits 4645 4647 +2
+ Misses 4512 4510 -2
Partials 2336 2336 ☔ View full report in Codecov by Sentry. |
@@ -537,10 +537,6 @@ describe('Dockstore my workflows part 3', () => { | |||
cy.visit('/my-workflows/github.com/A/l'); | |||
isActiveTab('Info'); | |||
tabs.forEach((tab) => { | |||
if (tab === 'Tools') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Part of revert commit to the way it was
@@ -23,3 +23,9 @@ a.underline-none { | |||
width: 14rem; | |||
text-align: start; | |||
} | |||
|
|||
:host ::ng-deep .mat-tab-label { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change that makes the tabs narrower
@@ -374,10 +374,18 @@ <h3> | |||
</ng-template> | |||
</ng-template> | |||
</mat-tab> | |||
<mat-tab id="metricsTab" label="Metrics"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All changes in this file are a revert
@@ -6,7 +6,6 @@ describe('Dockstore Metrics', () => { | |||
setTokenUserViewPort(); | |||
it('Should see no metrics banner', () => { | |||
cy.visit('/workflows/github.com/A/l:master'); | |||
cy.get('.mat-tab-header-pagination-after').click(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pagination control no longer present, so don't click it. Applies to next to changes as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have wanted this change for a long time. For people with smaller screens, or that use a narrower browser window, it is a big usability improvement. A little nerve-wracking to introduce it so close to the release date, but it'll probably be ok...
|
||
:host ::ng-deep .mat-tab-label { | ||
padding-left: 0.5em !important; | ||
padding-right: 0.5em !important; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initially, I thought it might make sense to increase the total minimum padding to a little more than 1em (0.5em + 0.5em). However, in the "super narrow screen" scenario, it's probably preferable to have the options jammed together, even though it's a tad ugly, rather than need to use a button to see them. So, this is good.
Description
Rolls back #1926, which reordered the tabs to make the Metrics tab visible without scrolling.
Instead make the tabs narrower, so they all fit, and no scrolling is required, per Steve's comment.
TLDR; restore original order of tabs on workflow component, tabs are now narrower to avoid scrolling.
Review Instructions
What it looks like with the fix, full screen and minimum width screen:
Issue
SEAB-6231
Security
If there are any concerns that require extra attention from the security team, highlight them here.
Please make sure that you've checked the following before submitting your pull request. Thanks!
npm run build
markdown-wrapper
component, which does extra sanitizationnpm audit
and ensure you are not introducing new vulnerabilities