-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Move console panel to bottom #20183
Move console panel to bottom #20183
Conversation
@@ -36,13 +36,13 @@ | |||
</Sidebar.Footer> | |||
</HcAppFrame::Sidenav> | |||
</Frame.Sidebar> | |||
<Frame.Main> | |||
<Frame.Main class="is-relative"> |
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.
😁
@@ -5,12 +5,13 @@ | |||
|
|||
.console-ui-panel { | |||
background: var(--token-color-palette-neutral-700); | |||
width: 100%; |
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.
if you want, definitely up to you, but you could use a helper class "is-fullwidth".
top: $spacing-xs; | ||
right: $spacing-xs; | ||
display: flex; |
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.
There are some helper classes for flex things. "is-flex" and "is-flex-end". Again, up to you.
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.
Sounds good. I'm not sure I see the benefit since there is a class already for the element. I think if this element only needed to be flexed I would definitely use helpers instead.
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.
Agreed, unless you were having issues with overriding (most helpers use !important) if a class exists then helpers are not as relevant.
<Icon @name="x" aria-label="Close console" /> | ||
</button> | ||
<div class="console-close-button"> | ||
<button type="button" class="button is-ghost" {{action "closeConsole"}} data-test-console-panel-close> |
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.
If you wanted to add a helper "has-right-margin-xs". There are two we use now "has-right-margin-m" and "has-right-margin-l". Totally up to you.
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.
Sounds good. I use those quite often but I typically will use helpers to avoid having to create a class. Do you think there is an advantage to removing the margin in favor of a helper here?
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 suppose in the case of the margin it could be removed since it's the only rule directly on the button but removing the flex rules from console-close-button
doesn't allow us to remove the class. Have we established a guideline on how much we should lean on helper classes as opposed to defined classes for an element and whether or not we should be mixing them?
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.
It's a great question. I'll add it to UI sync.
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.
LGTM. Just a couple of non-blocking comments. And don't worry everyone is getting the 🔬 from me on classes. It's just now that I know nearly all the classes by memory, I can't help it.
* Add Helios Design System Components (#19278) * adds hds dependency * updates reset import path * sets minifyCSS advanced option to false * Remove node-sass (#19376) * removes node-sass and fixes sass compilation * fixes active tab li class * Sidebar Navigation Components (#19446) * links ember-shared-components addon and imports styles * adds sidebar frame and nav components * updates HcNav component name to HcAppFrame and adds sidebar UserMenu component * adds tests for sidebar components * fixes tests * updates user menu styling * fixes typos in nav cluster component * changes padding value in sidebar stylesheet to use variable * Replace and remove old nav components with new ones (#19447) * links ember-shared-components addon and imports styles * adds sidebar frame and nav components * updates activeCluster on auth service and adds activeSession prop for sidebar visibility * replaces old nav components with new ones in templates * fixes sidebar visibility issue and updates user menu label class * removes NavHeader usage * adds clients index route to redirect to dashboard * removes unused HcAppFrame footer block and reduces page header top margin * Nav component cleanup (#19681) * removes nav-header components * removes navbar styling * removes status-menu component and styles * removes cluster and auth info components * removes menu-sidebar component and styling * fixes tests * Console Panel Updates (#19741) * updates console panel styling * adds test for opening and closing the console panel * updates console panel background color to use hds token * adds right margin to console panel input * updates link-status banner styling * updates hc nav components to new API * Namespace Picker Updates (#19753) * updates namespace-picker * updates namespace picker menu styling * adds bottom margin to env banner * updates class order on namespace picker link * restores manage namespaces refresh icon * removes manage namespaces nav icon * removes home link component (#20027) * Auth and Error View Updates (#19749) * adds vault logo to auth page * updates top level error template * updates loading substate handling and moves policies link from access to cluster nav (#20033) * moves console panel to bottom of viewport (#20183) * HDS Sidebar Nav Components (#20197) * updates nav components to hds * upgrades project yarn version to 3.5 * fixes issues in app frame component * updates sidenav actions to use icon button component * Sidebar navigation acceptance tests (#20270) * adds sidebar navigation acceptance tests and fixes other test failures * console panel styling tweaks * bumps addon version * remove and ignore yarn install-state file * fixes auth service and console tests * moves classes from deleted files after bulma merge * fixes sass syntax errors blocking build * cleans up dart sass deprecation warnings * adds changelog entry * hides namespace picker when sidebar nav panel is minimized * style tweaks * fixes sidebar nav tests * bumps hds addon to latest version and removes style override * updates modify-passthrough-response helper * updates sidebar nav tests * mfa-setup test fix attempt * fixes cluster mfa setup test * remove deprecated yarn ignore-optional flag from makefile * removes another instance of yarn ignore-optional and updates ui readme * removes unsupported yarn verbose flag from ci-helper * hides nav headings when user does not have access to any sub links * removes unused optional deps and moves lint-staged to dev deps * updates has-permission helper and permissions service tests * fixes issue with console panel not filling container width
This PR moves the console panel from the top of the viewport to the bottom to improve UX and give it a more natural feel.
Before
As you can see in the screenshot the close button is pushed up when the content scrolls. This has been fixed to anchor it to the top right of the panel.
After