Add `static` folder for static files and pre-loaded stylesheets (using new utility) #624

edan-bainglass · 2024-07-11T06:59:01Z

This PR adds a static folder to the widgets base package. At the moment, the folder holds stylesheets that are loaded (using a new utility function) on any import from the widgets base package.

codecov · 2024-07-11T07:43:08Z

Codecov Report

Attention: Patch coverage is 48.00000% with 13 lines in your changes missing coverage. Please review.

Project coverage is 83.49%. Comparing base (0c9f6fb) to head (7669080).
Report is 3 commits behind head on master.

Files with missing lines	Patch %	Lines
aiidalab_widgets_base/__init__.py	14.28%	12 Missing ⚠️
aiidalab_widgets_base/utils/loaders.py	90.90%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #624      +/-   ##
==========================================
- Coverage   83.61%   83.49%   -0.12%     
==========================================
  Files          16       17       +1     
  Lines        3522     3539      +17     
==========================================
+ Hits         2945     2955      +10     
- Misses        577      584       +7

Flag	Coverage Δ
python-3.11	`83.49% <48.00%> (-0.12%)`	⬇️
python-3.9	`83.53% <48.00%> (-0.12%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

danielhollas

Hi @edan-bainglass,

thanks so much on working on this, I am really excited to see this land.

My main concern here is about SCSS, and is twofold.

It introduces additional complexity of extra compilation step (see my other comments)
I guess that most of us are not so proficient with CSS, and SCSS is yet another cognitive overhead. Especially as we just start using CSS directly, my feeling is that we should start simple.

If you feel strongly about it, could you please open an issue (or topic on Discourse), and explain what are the advantages of SCSS (and what is the additional complexity) so we can discuss it a bit before committing?

aiidalab_widgets_base/__init__.py

aiidalab_widgets_base/utils/loaders.py

aiidalab_widgets_base/static/styles/README.md

edan-bainglass · 2024-07-18T14:26:53Z

@danielhollas thanks for your comments. Let's discuss further on Discourse.

edan-bainglass · 2024-08-16T09:48:50Z

@danielhollas I think I'll keep SCSS local and only commit/use the CSS files. In other words, no extra compilation step required for the developer if they wish to only work in CSS. Will update the PR shortly.

edan-bainglass · 2024-08-21T07:49:51Z

@danielhollas could you please take a look at why the browser tests are failing? I'm not sure I understand what's happening here.

danielhollas · 2024-08-21T12:46:41Z

@edan-bainglass The same failures are seen on the main branch so it is not related to this PR.

edan-bainglass · 2024-08-21T12:50:00Z

@danielhollas I see. Okay then. Please review 🙏🏻 I have a few PRs built on top of this one both here and in the QE app. I think all is well other than the tests. I've removed the SCSS part.

danielhollas

Just some small nits.

My main thing is that it would be nice to demonstrate this functionality to prove that is working by including some actual CSS.

Perhaps a nice candidate would be this code that defines CSS inline for tables in DictViewer.

aiidalab-widgets-base/aiidalab_widgets_base/viewers.py

Lines 104 to 113 in de5f0cc

    
                   self.value += """ 
        
                   <style> 
        
                       .df { border: none; } 
        
                       .df tbody tr:nth-child(odd) { background-color: #e5e7e9; } 
        
                       .df tbody tr:nth-child(odd):hover { background-color:   #f5b7b1; } 
        
                       .df tbody tr:nth-child(even):hover { background-color:  #f5b7b1; } 
        
                       .df tbody td { min-width: 300px; text-align: center; border: none } 
        
                       .df th { text-align: center; border: none;  border-bottom: 1px solid black;} 
        
                   </style> 
        
                   """

EDIT: I know that you have follow-up PRs that do this so feel free to ignore this.

aiidalab_widgets_base/__init__.py

aiidalab_widgets_base/utils/loaders.py

superstar54

Hi @edan-bainglass , thansk for the work.

It would be great that you add this in the docs to tell the developer how to use it in their app.
You need add your test in the test_notebooks.

aiidalab_widgets_base/__init__.py

aiidalab_widgets_base/utils/loaders.py

superstar54 · 2024-08-21T15:49:12Z

tests/test_loaders.py

+def test_load_css_stylesheet():
+    """Test `load_css_stylesheet` function."""
+    package = "aiidalab_widgets_base.static.styles"
+    load_css_stylesheet(package=package, filename="global.css")
+    load_css_stylesheet(package=package)


The test here will not work because it is not running inside a notebook. To test whether the CSS works, you need to add a test in test_notebooks/test_notebooks.py.

Are you saying to add a test to test_notebooks.py, or to move this test to test_notebooks.py? Also, could you explain why exactly it doesn't work?

Yes, adding a new test in the test_notebooks.py, and check if the CSS is indeed loaded in the selenium_driver.

Also, could you explain why exactly it doesn't work?

the test.py file is not running in the notebook, thus the display(Javascript... code will not run. You can add some wrong code inside it, and the test will not throw error.

For the test in selenium, for example, you create a vbox, and attach a class, then in your testing CSS file, you change its color, and in selenium_driver, you try to find the vbox, and verify the color.

The test here is fine, I think but I agree it would be nice to have *an additionaltest intest_notebooks` that runs an actual browser via selenium that would prove that the CSS is indeed loaded and applied to a given element (I don't actually know if selenium allows that but I would expect so).

Yes, in the test notebook, you need at least

import aiidalab_widgets_base

I mean you don't need (shouldn't ) call load_css_stylesheet explicitly. Only need to import aiidalab_widgets_base, since it will call load_css_stylesheet in the __init__.py file.

I've loaded aiidalab_widgets_base in the test.ipynb notebook with a comment regarding its use, as it is not actually used explicitly.

Okay, notebook test passes 👍

danielhollas · 2024-08-21T17:32:48Z

Matter of fact, I could use a test case for a notebook test, as recommended by @superstar54.

Yep! 👍

So to clarify, the bit of code you mentioned (df styles) can be removed once I add it via css, correct?

That would be my expectation, yes! By the way, please don't use the df class as a selector, since it comes from pandas I believe and I am planning to substitute pandas for tabulate in near future. Please add a custom awb class for this.

edan-bainglass · 2024-08-21T17:35:47Z

@danielhollas proceeding with a made-up class for testing purposes only, as to not interfere with the code base for now.

edan-bainglass · 2024-08-22T05:26:09Z

I think this is good now? Failed tests unrelated I believe.

superstar54

LGTM! Thanks for the nice work!

danielhollas · 2024-08-22T11:01:33Z

notebooks/test.ipynb

@@ -0,0 +1,68 @@
+{


Let's rename this file, current name is way to vague. I would vote for test_css.ipynb. If we want to reuse it for other tests in the future we can rename it later to test_misc.py.

tests_notebooks/test_notebooks.py

aiidalab_widgets_base/static/styles/test.css

aiidalab_widgets_base/utils/loaders.py

edan-bainglass · 2024-08-29T10:43:36Z

Found some oversights in my code. Fixed it. Still not passing the tests. But its not just my test. @danielhollas @superstar54 could you please have a look?

danielhollas · 2024-08-29T11:03:49Z

@edan-bainglass have you tried running the tests locally? That should make it clearer why they are failing.

edan-bainglass · 2024-08-29T11:40:25Z

I couldn't figure out how to run selenium tests locally. I'll give it another go.

danielhollas · 2024-08-29T13:02:17Z

@yakutovicha might have some tips how to run the notebook tests locally.

Perhaps check the selenium docs, or pytest-selenium docs, there might be some browser dependencies to make it work.

danielhollas · 2024-08-30T15:14:00Z

@edan-bainglass congrats on merging this. 🎉 I'll make a new alpha release that you can then use on PRs on QEApp.

Could you please write here what you did to fix the tests and if / how you were able to test notebooks locally? I'll then add it to README for contributors. Thanks!

edan-bainglass · 2024-08-30T15:34:17Z

@danielhollas thanks 🙏 Though I'm really not sure what actually fixed it. I was unable to run selenium tests locally in the container. I just applied the ol' step-by-step approach, walking back my commits, pushing chunks at a time and observing the tests. The CSS loading on import using the new utility passed the tests. Adding the notebook test in the notebooks folder with the test CSS as part of the module passed the tests. Then finally migrating the test notebook and CSS to the notebook tests folder... also passed the tests 🤷‍♂️ So in the end, I actually changed nothing from the state at which tests were failing. This was very confusing in general, as it was tests unrelated to my own that were failing before. Something to do with the selenium driver itself not finding the elements stated by the tests. According to this article, "NoSuchElementException is one of the most common exceptions in Selenium WebDriver..."

Of course, happy to get another set of eyes on it. Maybe I missed something.

Replaced module loading with using direct path in aiidalab#624

This PR refactors the main notebook as follows: - Much of the notebook is refactored as an `AppWrapper` component holding the banner, main, and footer components - The banner component (used to be the welcome message) is redesigned as follows: - App logo (same as documentation logo) and subtitle - Toggle buttons triggering an info box optionally showing one of the following: - Guide section - the old welcome page plus links to online docs, tutorials, etc. - About section - general information about the app and acknowledgements - The actual app is now injected from the notebook into the main component. - The footer component showing copyrights and version stays at the bottom of the page. The PR also introduces the use of CSS, loading custom stylesheets in the notebook via the AWB CSS loader utility (aiidalab/aiidalab-widgets-base#624)

edan-bainglass assigned superstar54 Jul 11, 2024

edan-bainglass force-pushed the static branch from 2db21d2 to 7a36396 Compare July 11, 2024 07:38

edan-bainglass marked this pull request as draft July 11, 2024 08:54

edan-bainglass force-pushed the static branch from dfd4b27 to 16b9ab1 Compare July 11, 2024 09:55

edan-bainglass mentioned this pull request Jul 11, 2024

Add info box widgets with CSS stylesheets #623

Closed

edan-bainglass marked this pull request as ready for review July 11, 2024 10:00

danielhollas requested changes Jul 18, 2024

View reviewed changes

edan-bainglass force-pushed the static branch from 16b9ab1 to 3e0fd0a Compare August 21, 2024 07:30

edan-bainglass requested a review from danielhollas August 21, 2024 07:31

edan-bainglass force-pushed the static branch from 3e0fd0a to ceda522 Compare August 21, 2024 07:32

edan-bainglass force-pushed the static branch from ceda522 to 52a6938 Compare August 21, 2024 12:57

danielhollas requested changes Aug 21, 2024

View reviewed changes

edan-bainglass mentioned this pull request Aug 21, 2024

Implement path for in-app guides aiidalab/aiidalab-qe#763

Open

superstar54 requested changes Aug 21, 2024

View reviewed changes

edan-bainglass force-pushed the static branch 2 times, most recently from 7e7fd31 to f859695 Compare August 22, 2024 05:11

superstar54 self-requested a review August 22, 2024 05:45

superstar54 approved these changes Aug 22, 2024

View reviewed changes

danielhollas requested changes Aug 22, 2024

View reviewed changes

danielhollas reviewed Aug 22, 2024

View reviewed changes

aiidalab_widgets_base/utils/loaders.py Outdated Show resolved Hide resolved

edan-bainglass force-pushed the static branch from 8613744 to 0b1392d Compare August 22, 2024 20:24

edan-bainglass force-pushed the static branch from e635fbb to 8e6c303 Compare August 29, 2024 10:36

edan-bainglass marked this pull request as draft August 30, 2024 08:03

edan-bainglass force-pushed the static branch from 8e6c303 to fd643e7 Compare August 30, 2024 08:04

edan-bainglass mentioned this pull request Aug 30, 2024

Reorganize static folder and add pre-loaded stylesheets aiidalab/aiidalab-qe#772

Merged

edan-bainglass force-pushed the static branch 3 times, most recently from c3880d0 to 61f9792 Compare August 30, 2024 10:36

Simplify CSS loading utility

a88a81d

edan-bainglass force-pushed the static branch 6 times, most recently from 7c8e259 to d5c5086 Compare August 30, 2024 13:20

Add notebook test for CSS loader

f27a826

edan-bainglass force-pushed the static branch from d5c5086 to f27a826 Compare August 30, 2024 13:49

Document use of CSS

7669080

edan-bainglass marked this pull request as ready for review August 30, 2024 14:54

edan-bainglass merged commit 73df986 into aiidalab:master Aug 30, 2024
11 checks passed

danielhollas added the enhancement New feature or request label Sep 1, 2024

edan-bainglass added a commit to edan-bainglass/aiidalab-widgets-base that referenced this pull request Sep 2, 2024

Discard module file from static styles folder

6b2d18c

Replaced module loading with using direct path in aiidalab#624

edan-bainglass mentioned this pull request Sep 2, 2024

Discard module file from static styles folder #632

Closed

edan-bainglass mentioned this pull request Sep 17, 2024

Redesign header with logo, toggled info sections, etc. aiidalab/aiidalab-qe#751

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `static` folder for static files and pre-loaded stylesheets (using new utility) #624

Add `static` folder for static files and pre-loaded stylesheets (using new utility) #624

edan-bainglass commented Jul 11, 2024 •

edited

Loading

codecov bot commented Jul 11, 2024 •

edited

Loading

danielhollas left a comment

edan-bainglass commented Jul 18, 2024

edan-bainglass commented Aug 16, 2024

edan-bainglass commented Aug 21, 2024

danielhollas commented Aug 21, 2024

edan-bainglass commented Aug 21, 2024

danielhollas left a comment •

edited

Loading

superstar54 left a comment

superstar54 Aug 21, 2024

edan-bainglass Aug 21, 2024

superstar54 Aug 21, 2024

superstar54 Aug 21, 2024

danielhollas Aug 21, 2024

superstar54 Aug 21, 2024

edan-bainglass Aug 21, 2024

superstar54 Aug 21, 2024 •

edited

Loading

edan-bainglass Aug 22, 2024

edan-bainglass Aug 22, 2024

danielhollas commented Aug 21, 2024

edan-bainglass commented Aug 21, 2024

edan-bainglass commented Aug 22, 2024

superstar54 left a comment

danielhollas Aug 22, 2024

edan-bainglass commented Aug 29, 2024

danielhollas commented Aug 29, 2024

edan-bainglass commented Aug 29, 2024

danielhollas commented Aug 29, 2024

danielhollas commented Aug 30, 2024

edan-bainglass commented Aug 30, 2024

	self.value += """
	<style>
	.df { border: none; }
	.df tbody tr:nth-child(odd) { background-color: #e5e7e9; }
	.df tbody tr:nth-child(odd):hover { background-color: #f5b7b1; }
	.df tbody tr:nth-child(even):hover { background-color: #f5b7b1; }
	.df tbody td { min-width: 300px; text-align: center; border: none }
	.df th { text-align: center; border: none; border-bottom: 1px solid black;}
	</style>
	"""

Add static folder for static files and pre-loaded stylesheets (using new utility) #624

Add static folder for static files and pre-loaded stylesheets (using new utility) #624

Conversation

edan-bainglass commented Jul 11, 2024 • edited Loading

codecov bot commented Jul 11, 2024 • edited Loading

Codecov Report

danielhollas left a comment

Choose a reason for hiding this comment

edan-bainglass commented Jul 18, 2024

edan-bainglass commented Aug 16, 2024

edan-bainglass commented Aug 21, 2024

danielhollas commented Aug 21, 2024

edan-bainglass commented Aug 21, 2024

danielhollas left a comment • edited Loading

Choose a reason for hiding this comment

superstar54 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

superstar54 Aug 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danielhollas commented Aug 21, 2024

edan-bainglass commented Aug 21, 2024

edan-bainglass commented Aug 22, 2024

superstar54 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edan-bainglass commented Aug 29, 2024

danielhollas commented Aug 29, 2024

edan-bainglass commented Aug 29, 2024

danielhollas commented Aug 29, 2024

danielhollas commented Aug 30, 2024

edan-bainglass commented Aug 30, 2024

Add `static` folder for static files and pre-loaded stylesheets (using new utility) #624

Add `static` folder for static files and pre-loaded stylesheets (using new utility) #624

edan-bainglass commented Jul 11, 2024 •

edited

Loading

codecov bot commented Jul 11, 2024 •

edited

Loading

danielhollas left a comment •

edited

Loading

superstar54 Aug 21, 2024 •

edited

Loading