ThreeSixtyGiving / grantnav

This is a web based search tool for data in the 360 giving data format.
http://grantnav.threesixtygiving.org/
Other
9 stars 5 forks source link

Widgets - minor changes #873

Closed mariongalley closed 1 year ago

mariongalley commented 2 years ago
Lathrisk commented 2 years ago

@mariongalley There were a few things going on in the suggestions above so I have mixed them into something cohesive, but more than happy to rework this if it has diverge from your vision of it.

These changes remove the grants table title, the info bar at the bottom, and the CSV/JSON download buttons. These are replaced with a header that contains the title, and a link to grantnav which includes the logo. This header is styled based on the summary header insights link on the main search page.

EDIT: Would you prefer the CTA style rather than the logo link and large header?

Screenshot from 2022-10-26 15-20-49

mariongalley commented 2 years ago

There were a few things going on in the suggestions above so I have mixed them into something cohesive, but more than happy to rework this if it has diverge from your vision of it.

@Lathrisk No I think this looks great, thank you! I've ticked off the things that are done, but I couldn't ascertain whether the width and the background colour ones were done based off this screenshot - you can tick them off if they're done.

This header is styled based on the summary header insights link on the main search page.

EDIT: Would you prefer the CTA style rather than the logo link and large header?

I really like having a nice header that clearly states that the data is from GrantNav and how to go there. However, I don't think we can put the logo over a yellow background as the contrast with the yellow dots is insufficient (and there's something in our brand guidelines about it, probably!)

I don't in principle object to the colours from the search summary being used if we can work out a way to give the logo a white background that looks nice. Otherwise we can style it more like other headers we have e.g. use a white background with text in the 360Giving colours and a light grey divider - but less tall! image

Lathrisk commented 2 years ago

@mariongalley I've removed the summary box styling and used the nav style that we would find in the data quality tool to give a relatively concise header. The logo and the text both link to the same search results currently. I've attached screenshots for all of the existing widgets with a selection of visual improvements. These are all responsive and will resize based on the size of the containing iframe, I need to test the limits of that though.

Let me know any feedback.

Screenshot from 2022-10-27 13-08-11 Screenshot from 2022-10-27 13-08-04 Screenshot from 2022-10-27 13-07-53

Lathrisk commented 2 years ago

I've pushed the latest changes to the dev site. I think the responsiveness of the content needs tweaking so that we can remove the scroll bar.

http://grantnav.360dev1.vs.mythic-beasts.com/widgets?query=potato&default_field=%2A&sort=_score+desc

mariongalley commented 2 years ago

@Lathrisk This is fantastic, thank you! I'll do a bit of testing today and let you know if I have any other feedback.

If I do I suspect it won't be a blocker to releasing this alongside Org pages, subject to security assurances and obfuscation of the configuration screen.

Lathrisk commented 2 years ago

@mariongalley Great, thanks. I have done some of the security work, so that only the widgets and API endpoints are exempt from the cross-site vulnerabilities. This still presents a risk, but I think the only additional mitigation would be tracking all of the sites that the widgets are installed on and whitelisting those URLs. This could be done, in theory, but it is far from a trivial task.

We might need to think about the best route for obfuscation or password protection. Django has some auth/password functionality, but I'm not sure how complex it is for our use case.

mariongalley commented 2 years ago

@Lathrisk done some more testing, it looks like the widget builder is still providing HTML with a defined fixed rather than relative width, so I'm not sure if that means point 1 isn't ticked off?

We might need to think about the best route for obfuscation or password protection. Django has some auth/password functionality, but I'm not sure how complex it is for our use case.

Happy with either, whatever you think is preferable.

mariongalley commented 1 year ago

@codemacabre The only task remaining is to remove the fixed width and set it to 100% so the parent container can define the width & allow selecting the width of the iFrame in the widget builder.

codemacabre commented 1 year ago

@mariongalley Is this issue still outstanding? It appears to work when tested locally as the width has already been set to 100% here: https://github.com/ThreeSixtyGiving/grantnav/blob/5592df7cb2ee9167f31dab37c027db066c19bd55/grantnav/frontend/templates/search_widgets.html#L205

mariongalley commented 1 year ago

@codemacabre Setting width to 100% is done but there's a nice-to-have about allowing the width to be configurable, and @michaelwood suggested the approach above for that

michaelwood commented 1 year ago

So now that the widget is 100% of the iframe I was thinking people could specify the px size of the iframe, so that when they embed it it will fit in a particular area of a page etc.

Was thinking about the twitter example:

<iframe id="twitter-widget-0" scrolling="no" frameborder="0" allowtransparency="true" allowfullscreen="true" class="" style="position: static; visibility: visible; width: 520px; height: 600px; display: block; flex-grow: 1;" title="Twitter Timeline" ...></iframe>
codemacabre commented 1 year ago

This should be live now.

mariongalley commented 1 year ago

Looks good, thanks @codemacabre !