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

New attribute page_img: logos for for pages and sections #290

Open
wants to merge 39 commits into
base: main
Choose a base branch
from

Conversation

PhilReedData
Copy link
Contributor

@PhilReedData PhilReedData commented Oct 11, 2024

As discussed, I have borrowed the image page attribute from RO-Crate and used a new page_img attribute for pages to show a graphic such as a logo to the side of the page title. It also shows on the section navigation cards. There is no change to the rendered HTML in either place unless the page_img attribute appears in the front-matter of a page.

More details and images are described in a Google Doc.

This has been tested on a branch of the FAIRDOM website, and in the ETT site itself. I used the following code in _config.yml to make a remote call:
remote_theme: PhilReedData/elixir-toolkit-theme@page-img-007

This is a fork from version 3.2.0. I did not mean to change the Gemfile.lock file.

To do

  • Further testing on other sites.
  • Tweak the image sizes for mobile viewport and particularly wide/narrow images.
  • Perhaps a flag to turn the feature off in section navigation and/or pages.

Sample images

More images in the linked Google Doc above.
image
image
imageimage
image
image

@bedroesb
Copy link
Member

@PhilReedData Hey! Sorry it took so long to look at your pull request. I played around with some bootstrap classes. Could you have a look in how it behaves on your deployment?

@PhilReedData
Copy link
Contributor Author

Thanks, I certainly will try that locally. Which remote call should I use now? To a particular tag/release of ETT?

@bedroesb
Copy link
Member

this is your branch still !

@bedroesb
Copy link
Member

@PhilReedData remote_theme: PhilReedData/[email protected]

@PhilReedData
Copy link
Contributor Author

Excellent, thank you. This is working really well for me.
image
image

image
image

@bedroesb
Copy link
Member

bedroesb commented Nov 22, 2024

I don't think you have pulled the latest version of your branch? can you check?

Sorry something behaved different than I expected, you do have the latest version , let me check!

@PhilReedData
Copy link
Contributor Author

Ah okay. I thought I had a later version because of the way the logo appears above the heading on the mobile view, that's changed from my code (changed for the better).

@bedroesb
Copy link
Member

@phil, can you point me to a place/branch where you try out the FAIRDOM Seek website with this version of the theme? The second screenshot show the logo to be be squeezed for example. and I think I want to play a little bit more with the spacings.

@PhilReedData
Copy link
Contributor Author

@bedroesb
Copy link
Member

bedroesb commented Dec 4, 2024

@PhilReedData I just discovered where I got confused. You are manually overwriting on the FAIRDOM website, the _includes/section-navigation-tiles.html. If you delete that one you can see my new version.

@PhilReedData
Copy link
Contributor Author

For some reason, the HITS project page wouldn't show its logo until I added a few subheadings (h2) into the page. The others are working fine, showing the image regardless of toc true/false.

image
^ when there is no h2 content in the page

image
^ when there is no h2 content in the page and toc: false

image
^ when I add at least one h2 line

image
^ when I add at least one h2 line and toc: false

Is there a bug?

@PhilReedData
Copy link
Contributor Author

The FAIRDOM Communities page, it has some h2 headings, and toc: false, and it adds a blank right column anyway that was not there before.
image
I am adding a TOC there now anyway, but it looks like an unintended side effect of this work.

@PhilReedData
Copy link
Contributor Author

That has worked, thanks!
image

@bedroesb
Copy link
Member

Good catch btw. That was a bug. I think I am only now happy with all user scenarios. Sorry for all the changes! My conditional was not behaving as expected...

@PhilReedData
Copy link
Contributor Author

No worries, it is good to catch these things now.
One more possible bug, when I mouse over the section title, I have a CSS animation like with the RO-Crate site. It reflows the text which briefly runs onto another line, then back again. I think this has happened since adding the page image to the tile.
ETTanimation
I have only seen this with one of our tiles, it happens to be the right length of text to be borderline whether a new line is required or not. Do you have any thoughts on this?

@PhilReedData
Copy link
Contributor Author

Similar with one other tile, but it doesn't resize.
ETTanimation2

@bedroesb
Copy link
Member

bedroesb commented Dec 10, 2024

@PhilReedData THis is unrelated to the changes here, but is related to the hover effect put in place. If the description is is causing an edge case where it is longer that the width it has + a small change in width available, the text wrapping will get triggered. This might not have been the case in the past, since there was more width available since the logo was not there. But I can give you an example where this is already the case: https://www.infectious-diseases-toolkit.org/showcase/ first tile.

You will have to play around with the hover effect you've set in your css.

@bedroesb
Copy link
Member

I think put a slight delay on the shrink of the width compared to the growth on the right side, could fix this.

@PhilReedData
Copy link
Contributor Author

Ah I see, thanks anyway. I had a look but I'm already using the same code as your example. It is a rare problem, only happens at specific widths, and if you say it's not been introduced by the page image work then I'll leave it alone for now.

What comes next? Is this work ready to go in the next ETT release, and should I wait for that before merging into the production FAIRDOM website?

@bedroesb
Copy link
Member

I could still Iron out some behaviours while testing it on other instances

@bedroesb
Copy link
Member

@PhilReedData I think we are kind of in a good state? I tried in in the ELIXIR toolkit theme, the FAIRDOM website (with and without toc), RDMkit and Infectious diseases toolkit. Tried out multiple breaking points. If you could maybe check out on Safari.

@bedroesb
Copy link
Member

@PhilReedData once I have released a new version, you can use that one in your FAIRDOM instance!

@PhilReedData
Copy link
Contributor Author

Here are my Safari tests with FAIRDOM:
image
image
image
image

image
^ FAIRDOMHub page is not showing the image in Safari, but is in Chrome.
image
More precisely, the image flashes up for less than a second at load.

When I take out toc: false, the image disappears on both.
image
image

If I add a (temporary) h2, the image reappears on Chrome only.
image
image

This problem does not happen on the SEEK page, which is otherwise the same (toc: false).
Perhaps the choice of image on the FAIRDOMHub page is a factor.

@PhilReedData
Copy link
Contributor Author

Good news, restarting my local server has fixed the problem serving the FAIRDOMHub logo, strangely.
image
I've noticed that there is more white space before the footer now.
image
Testing the middle breakpoint.

@bedroesb
Copy link
Member

@PhilReedData I think this will be a caching thing, since I had to update some javascript and that does not get a unique identifier. Let me improve that!

@bedroesb
Copy link
Member

Normally this should not happen anymore

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.

Allow the inclusion of page image metadata to be displayed in the section navigation tiles
2 participants