Skip to content
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

Responsive redesign #55

Merged
merged 51 commits into from
Jul 28, 2023
Merged

Responsive redesign #55

merged 51 commits into from
Jul 28, 2023

Conversation

themoenen
Copy link
Contributor

Major refactoring

  • Responsiveness: the grid as well as all UI components are now fully responsive, up until around ~415px wide. Smaller usage seems unlikely.

  • Iframe usage:

    • We no longer render the grid inside an iframe by default.
    • When used inside of Jupyter Notebook or Labs, we default back to the use of an iframe, because multiple grids will be rendered on a single page.
    • A new parameter use_iframe has been added to override the default behavior.
    • Parameters width and height have been renamed to iframe_width and iframe_height to more descriptive.
  • Templates: the pages / table templates and corresponding functions have been renamed to interactive / static for clarity.

  • Interactive navigation components (pagination, sort, search, select menu):

    • Removed bootstrap CSS & rebuilt components as native elements with minimal, name-spaced CSS.
      Note: we're still relying on bootstrap JS for the tooltip.
    • Improved the sorting UI to be more intuitive.
    • You can now toggle between text/smarts search instead of having to click a dropdown.
    • The checkbox menu icon was replaced with a more standard triple dot menu.
    • All UI elements are now neatly aligned and displayed on top so they're accessible without scrolling.
  • Grid improvements:

    • The mols2grid-id is now permanently displayed next to the checkbox in the corner.
    • When the gap parameter (which controls the gap between cells) is set to 0, a negative margin ensures we avoid double borders in between cells, resulting in a tighter look.
    • Added a new parameter pad for controlling the padding of each cell.
    • Interactive template:
      • The entire cell is now clickable, instead of just the tiny checkbox.
      • Implemented some basic keyboard navigation: ENTER / ESC for selecting / unselecting, / for navigation.
      • Instead of specifying how many columns, you just specify a desired image size. This width is then also used to set a minimum cell width.
      • The n_items_per_page parameter has been added, replacing n_cols for the interactive template.
      • The responsive CSS assumes results to be a multiple of 12, and will elegantly snap to 1/2/3/4/6/12 columns.
        • By default, 24 results are returned. The documentation suggests sticking to multiples of 12.
        • When n_items_per_page is set to a different number, the grid will just display a gap at the end of the page.
    • Static template:
      • Instead of specifying a grid width, the user can now just specify the image/cell width size[0] and the number of columns n_cols.
        This way there's optimal overlap with how the interactive grid works.
  • Data display:

    • Property values displayed via subbet or tooltip can now be copied by clicking them.
    • Longer property values are now only truncated in the interactive grid, and broken into multiple lines by default in the static grid, as it is mostly meant for printing.
    • A new parameter truncate lets you override the default truncating behavior.
      Note: when truncate is set to False, longer values will distort the grid, causing a more chaotic layout, but otherwise this doesn't affect the functionality.
  • Tooltip

    • Static template:
      • The tooltip_trigger default value is now focus.
      • Table cells are now focusable, so you can either click inside/outside a cell to activate/deactivate, or use TAB key to cycle through the grid.
    • Interactive template:
      • The tooltip_trigger parameter has been removed, the tooltip is now controlled by JS (See initToolTip in grid_interaction.js).
      • The tooltip hover zone is now limited to a small (i) icon in the corner, so it is not interfering with regular browsing.
      • Clicking the (i) icon toggles the tooltip and indicates if it's on or off, making it easier to find where a sticky tooltip is coming from.
      • The tooltip display zone (around which the tooltip is displayed) is now the entire cell instead of only the image, so it never overlaps with any of the cell's data or functionality.
  • Popup

    • The popup functionality has been fully decoupled from Bootstrap, simplifying the HTML. As a result, max-width: 80% is no longer needed as default styling.

Small improvements

  • We're now automatically adding img to the subset without throwing an error (previous behavior: throw error and tell user to add it.)
  • You can still pass img into the subset list, if you want to change the order of the data parameters and the image.
  • A smaller default font size (12px) makes the experience out of the box a bit more practical.
  • The molecule SVG images are now transparent, resulting in a nicer hover state.
  • Updated documentation:
    • Default values are now mentioned for all parameters.
    • Parameters are organized into categories.
    • Removed some description inconsistencies.
  • Save CVS: When exporting as CSV we now use a semicolon ; as delineator instead of a tab \t. This way CSVs are properly previewed on Mac.
  • Copy to clipboard: this will now record a tab delineated text which is ready to be pasted into a spreadsheet, instead of the less useful dictionary format.

Bug fixes

  • When you download a CSV or SMILES file without any cells selected, you will now download data for all cells instead of an empty document.
  • Parameter gap in static template didn't work.
  • We no longer resize the iframe if a custom iframe_height is set by the display function.
    • Previous behavior: parameter was overwritten by the style tag, hence useless.
    • Also removed the default height="200" because it didn't serve any purpose.
  • Removed the cell_width parameter as it was being overwritten and not documented.
  • Removed from the custom_css parameter instructions 'Only available for the "interactive" template' because it's not.

@cbouy
Copy link
Owner

cbouy commented Jul 22, 2023

I fixed some of the tests but there are a couple more that I need to go through so the pipeline might still fail, will try to go through the rest either monday or wednesday

@cbouy cbouy force-pushed the master branch 2 times, most recently from 2a462ac to cb4ec47 Compare July 27, 2023 00:52
@codecov
Copy link

codecov bot commented Jul 27, 2023

Codecov Report

Merging #55 (07f3aad) into master (2d3f2c9) will decrease coverage by 0.30%.
The diff coverage is 92.30%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #55      +/-   ##
==========================================
- Coverage   90.43%   90.14%   -0.30%     
==========================================
  Files          10       10              
  Lines         523      538      +15     
==========================================
+ Hits          473      485      +12     
- Misses         50       53       +3     

Impacted file tree graph

Files Changed Coverage Δ
mols2grid/molgrid.py 92.41% <91.66%> (-0.57%) ⬇️
mols2grid/callbacks.py 95.45% <100.00%> (ø)
mols2grid/dispatch.py 60.71% <100.00%> (ø)
mols2grid/utils.py 98.11% <100.00%> (ø)

@cbouy
Copy link
Owner

cbouy commented Jul 27, 2023

Okay the test pipeline should pass now and the docs build as well.
I just have two nitpicky comments which are related:
image

  • The info hover button has no background, border, and does not "invert" colors when being hovered, but the callback one has all this. Could it be possible to make the info button similar to the callback one? I think the border and color inversion makes it more clear that you can also click the info button to anchor the tooltip.
  • For dark themes, the new default transparent background makes it hard to see the molecules (and the info button as well but addressing the comment above would fix it). Could it be worth adding a white background right where the molecules are displayed by default (possibly controlled by a new parameter)? I can take care of exposing and documenting the new parameter if that's okay with you.

@cbouy
Copy link
Owner

cbouy commented Jul 28, 2023

Edit: I took care of the background, I still need to update a few things (updating the changelog and the demo image, hopefully today) before merging. I'll take care of the info button update as well this weekend if I have time, then merge and prerelease so that people can give it a go before the actual release!

@cbouy cbouy merged commit 6d68c08 into cbouy:master Jul 28, 2023
6 checks passed
@cbouy
Copy link
Owner

cbouy commented Jul 28, 2023

Thanks again @themoenen for the awesome contribution!

@themoenen
Copy link
Contributor Author

I'll make sure to implement the info button feedback, but I may wrap this up with some other improvements that come out of testing later, so for now I will have to put this aside.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants