-
Notifications
You must be signed in to change notification settings - Fork 12
Expand Pipeline Viewport #1380
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
base: main
Are you sure you want to change the base?
Expand Pipeline Viewport #1380
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. 📢 Thoughts on this report? Let us know! 🚀 New features to boost your workflow:
|
const sidebar = document.querySelector('.w-64'); | ||
const icon = document.getElementById('fullscreen-enter-icon'); | ||
|
||
function toggleFullScreen() { |
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.
have you tried doing this using alpineJS? You might find it easier and less verbose
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.
Yes gave it a shot you can check my latest commit.
<div class="max-w-7xl mx-auto" id="pipelineBuilder"> | ||
<div class="flex-1 relative" id="pipelines-container"> | ||
<button class="fullscreen-toggle" title="Toggle Full Screen"> | ||
<svg class="w-5 h-5" fill="none" stroke="currentColor" viewBox="0 0 24 24"> |
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'd suggest using the fa-solid fa-expand
font awesome icon here and the fa fa-compress
icon for the "shrink" one. I think it will fit a bit better with the rest of the look
@@ -2,6 +2,8 @@ | |||
{% load static %} | |||
{% block page_head %} | |||
<link rel="stylesheet" href="{% static 'css/pipeline.css' %}"> | |||
<link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/6.5.1/css/all.min.css" integrity="sha512-..." crossorigin="anonymous" referrerpolicy="no-referrer" /> |
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.
{% endblock %} | ||
{% block page_js %} | ||
{{ parameter_values|json_script:"parameter-values" }} | ||
{{ default_values|json_script:"default-values" }} | ||
{{ node_schemas|json_script:"node-schemas" }} | ||
{{ flags_enabled|json_script:"flags-enabled" }} |
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.
This was probably removed by accident?
</div> | ||
<aside class="w-64 hidden" x-ref="sidebar">Sidebar content here</aside> |
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.
What is this side bar?
$watch('isFullscreen', (value) => { | ||
if (value) { | ||
$el.requestFullscreen().catch(err => console.error('Fullscreen failed:', err)); | ||
if (sidebar) sidebar.classList.add('hidden'); |
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 sidebar can be referenced using x-ref
. For instance, you can put x-ref=sidebar
on the sidebar element here. That said, in alpine there's an easy way to show/hide an element based on some value in data
, and that is to use x-show=!isFullscreen
(see the x-show docs).
In order to do this, you will have to put the data directive with the isFullscreen
value on a level that contains both the sidebar and app elements. I think an alpine store might come in more handy here, since the store allows components on different parts of the page to communicate with each other without having to share a data
directive. We can initialize the store in this file by including something like
<script>
document.addEventListener('alpine:init', () => {
Alpine.store('fullscreen', {
on: false,
toggle() {
this.on = ! this.on
}
})
})
</script>
in the script. Note that this will only make the store available on pipeline pages, so if we reference it elsewhere we'd have to check its availability first. Anyhoo, after this, you can easily create bindings by simply referencing the store's value. For example, to show/hide the sidebar, add the following attributes on the sidebar element:
x-data x-show="!$store.fullscreen || ($store.fullscreen && !$store.fullscreen.on)"
(It's important to keep the x-data
attirubte to indicate to alpineJS that this element wants to use Alpine).
P.S. If we don't want to check the store's availability first, we can always included the store code in the header in this file
Providing a full screen toggle mode for pipeline building as a part of - #1166
adding a demo for reference -
export.pipeline.viewport.mp4