-
Notifications
You must be signed in to change notification settings - Fork 25
Minor improvements to the XRD-insitu block front end #1412
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1412 +/- ##
=======================================
Coverage 80.04% 80.04%
=======================================
Files 70 70
Lines 4836 4836
=======================================
Hits 3871 3871
Misses 965 965 🚀 New features to boost your workflow:
|
datalab
|
||||||||||||||||||||||||||||
| Project |
datalab
|
| Branch Review |
bes/XRD_insitu_improvements
|
| Run status |
|
| Run duration | 07m 41s |
| Commit |
|
| Committer | Ben Smith |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
|
0
|
|
|
0
|
|
|
0
|
|
|
0
|
|
|
354
|
| View all changes introduced in this branch ↗︎ | |
e925242 to
9e61224
Compare
ml-evs
left a comment
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.
Please bump the version number of the plugin here after merging the LDE PR
BenjaminCharmes
left a comment
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, working great!
For consistency purposes, I think we should refactor this in a future PR to:
- Use Popper.js (like
StyledBlockInfoandStyledBlockHelp) instead of the nativetitleattribute, which has a slower delay (~1s) and doesn't match our UI styling - Add hover effects on the info icon (color transition)
- Use
cursor: pointerinstead ofcursor: helpfor consistency
Even better would be to create a generic HelpTooltip component that could be reused throughout the app and potentially replace the similar logic in StyledBlockInfo and StyledBlockHelp.
d355857 to
3032cd9
Compare
ml-evs
left a comment
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.
Let's merge this and implement @BenjaminCharmes's suggestions in a future PR!
Added info hover panels to data granularity and sample granularity. Added a box for File pattern so supply glob_str to the backend for matching patterns in the files. (E.g "summed" to grab all the summed mythen files for XRD data).
Default behaviour if nothing is added to File pattern is to match all files.