-
Notifications
You must be signed in to change notification settings - Fork 15
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
Improvement/bandswidget sizewarning #839
Improvement/bandswidget sizewarning #839
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #839 +/- ##
==========================================
+ Coverage 67.84% 67.87% +0.02%
==========================================
Files 50 50
Lines 4503 4507 +4
==========================================
+ Hits 3055 3059 +4
Misses 1448 1448
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Using CSS, you could modify the overflow-x property of the plot's container. This will add a horizontal scroll bar when the full container does not fit the screen. |
@edan-bainglass Something like this ? Now the question is showing the scrolling horizontal bar is enough , or if there is the need of the warning displayed as well. |
Looks good. Enough in my option. |
Now the question , is should we do it only for electronic structure , or include PDOS tab as well ? |
If possible, it would be good for all the widgets to have a scroll bar on a small screen. |
|
||
plot_container = ipw.HTML( | ||
""" | ||
<div style='overflow-x: auto; width: 80%; max-width: 80%;'> | ||
<!-- Placeholder for the Plotly plot widget --> | ||
</div> | ||
""" | ||
) | ||
_bands_dos_widget.layout = ipw.Layout(width="1000px") | ||
self.children = [ | ||
ipw.VBox( | ||
[ | ||
plot_container, # The scrollable container | ||
_bands_dos_widget, # The actual Plotly widget | ||
] | ||
) | ||
] |
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.
Add a layout with a fixed width should already add the scroll bar on a small screen.
plot_container = ipw.HTML( | |
""" | |
<div style='overflow-x: auto; width: 80%; max-width: 80%;'> | |
<!-- Placeholder for the Plotly plot widget --> | |
</div> | |
""" | |
) | |
_bands_dos_widget.layout = ipw.Layout(width="1000px") | |
self.children = [ | |
ipw.VBox( | |
[ | |
plot_container, # The scrollable container | |
_bands_dos_widget, # The actual Plotly widget | |
] | |
) | |
] | |
_bands_dos_widget.layout = ipw.Layout(width="1000px") | |
self.children = [ | |
_bands_dos_widget, # The actual Plotly widget | |
] |
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.
But then the bar is enough , or do we want a warning too ?
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.
Ah, the plot_container
is used for the warning message. Sorry, I misunderstood it.
I think the bar is enough.
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.
Thanks for the work! LGTM!
* Include an ipywidget Layout to the BandsPdosWidget to set an horizontal scroll bar
This PR address #820
The idea I have is to include a javascript that just check if the width and display a message if the size is too narrow.
I am not well knowledgable in JavaScript ( I got a lot of help on this one from ChatGPT) Please let me know any suggestions or ideas you might have in mind to address this issue ?
@edan-bainglass @superstar54