Add Hero widget #1077

rogerhutchings · 2019-08-14T15:46:19Z

Closes #1018
Closes #49 🎉

Set default screen size to 'small'
Update React version to match other apps
Add Hero component
- Add background
- Add about link and intro
- Add workflow loader (with translated strings!)

Mobile

Desktop

Review Checklist

General

Are the tests passing locally and on Travis?
Is the documentation up to date?
Is the changelog updated?

Apps

Does it work in all major browsers: Firefox, Chrome, Edge, Safari?
Does it work on mobile?
Can you rm -rf node_modules/ && yarn bootstrap and app works as expected?
Can you run a production build of the app?

srallen

I have a few minor comments on initial pass

...screens/ProjectHomePage/components/Hero/helpers/fetchWorkflowsHelper/fetchWorkflowsHelper.js

packages/app-project/src/screens/ProjectHomePage/components/Hero/HeroContainer.js

.../app-project/src/screens/ProjectHomePage/components/Hero/components/Background/Background.js

packages/app-project/src/helpers/GrommetWrapper/GrommetWrapperContainer.js

...ect/src/screens/ProjectHomePage/components/Hero/components/Background/BackgroundContainer.js

...-project/src/screens/ProjectHomePage/components/Hero/components/Introduction/Introduction.js

...src/screens/ProjectHomePage/components/Hero/components/Introduction/IntroductionContainer.js

.../src/screens/ProjectHomePage/components/Hero/components/WorkflowSelector/WorkflowSelector.js

...nts/Hero/components/WorkflowSelector/components/WorkflowSelectButton/WorkflowSelectButton.js

srallen · 2019-08-14T17:08:01Z

...nts/Hero/components/WorkflowSelector/components/WorkflowSelectButton/WorkflowSelectButton.js

+
+  const as = workflow.default
+    ? `${router.asPath}/classify`
+    : `${router.asPath}/classify/workflow/${workflow.id}`


Woohoo! This is going to make building out workflow selection easier I think having a project home page to actually use for manual testing.

I'm actually thinking that all workflows should be routed to /classify/workflow/${workflow.id}, and that /classify simply does a redirect.

/project/[owner]/[project]/classify/workflow/[workflow_id] becomes our new classify page

No ambiguity in what resource lives at the classify url

No duplicated content at different urls

Thoughts / feelings?

Yeah, I'm fine with /classify redirecting to /classify/workflow/:workflow_id each time, but this should be in an ADR (if it's not already).

Sounds good to me but /classify should redirect to the stored workflow ID from project preferences for logged-in volunteers, rather than just redirecting to the default workflow.

Yeah, the same ordering of selection would exist: user stored UPP workflow id --> project set workflow id in UPP --> project default --> random active selection. Any one of those would result in the id that gets redirected to.

Also - for the purposes of this PR, I propose leaving as is for now

I think you can get rid of the ternary here and always route to the workflow ID when linking to a specific workflow (assuming these links are the same as the 'Choose a workflow' links on the current home page.)

rogerhutchings · 2019-08-16T10:37:38Z

On propTypes, MST's author recommends using optional props; it appears as though components get instantiated before some props can be injected. Otherwise, we set a bunch of default falsey defaults ¯\(ツ)/¯

rogerhutchings · 2019-09-02T09:05:52Z

@srallen would you mind taking another look?

srallen

I have a few more comments. Thanks!

packages/app-project/src/helpers/GrommetWrapper/GrommetWrapperContainer.js

...-project/src/screens/ProjectHomePage/components/Hero/components/Introduction/Introduction.js

.../src/screens/ProjectHomePage/components/Hero/components/WorkflowSelector/WorkflowSelector.js

srallen · 2019-09-03T19:19:37Z

...nts/Hero/components/WorkflowSelector/components/WorkflowSelectButton/WorkflowSelectButton.js

+
+  const as = workflow.default
+    ? `${router.asPath}/classify`
+    : `${router.asPath}/classify/workflow/${workflow.id}`


Yeah, I'm fine with /classify redirecting to /classify/workflow/:workflow_id each time, but this should be in an ADR (if it's not already).

...ero/components/WorkflowSelector/components/WorkflowSelectButton/WorkflowSelectButton.spec.js

...ns/ProjectHomePage/components/Hero/helpers/fetchWorkflowsHelper/fetchWorkflowsHelper.spec.js

packages/lib-grommet-theme/src/index.js

.../app-project/src/screens/ProjectHomePage/components/Hero/components/Background/Background.js

packages/app-project/src/screens/ProjectHomePage/components/Hero/HeroContainer.spec.js

packages/app-project/src/screens/ProjectHomePage/components/Hero/Hero.spec.js

- Set default screen size to 'small' - Update React to match other apps - Add Hero component - Add background - Add about link and intro - Add workflow loader

- Filter workflows response to only include requested fields - Filter workflows response to only include incomplete workflows - Move translations request to work in series, as `activeWorkflows` may include redundant translations and these can get big - Create a map of display names, rather than do an array search for performance

* Make Hero widget responsive with SSR - Install @artsy/fresnel - Move grommet theme merger to its own helper - Add fresnel helper components - Add media queries to Hero widget - Update tests * Update packages/app-project/src/helpers/theme/README.md Co-Authored-By: Jim O'Donnell <jim@zooniverse.org> * Add per-layout tests

rogerhutchings · 2019-09-13T10:12:25Z

okay, i've added the final set of tests - thanks to @srallen and @eatyourgreens for the help

rogerhutchings · 2019-09-18T14:59:07Z

With #1107 merging, this is going to need to change (specifically the default /classify route behaviour), but that can go in a separate PR.

beckyrother · 2019-09-18T15:08:19Z

@rogerhutchings the > is quite large, any way to adjust so it's the same height as the 'learn more' text? Other than that design looks great

srallen

LGTM. I have one non-blocking inline comment for the error state test. Additionally, I'm wondering about the Img component used to render the background. I had written the Media component in the shared components library to use the thumbnail service because we found that some projects were upload massively sized media. I think we have limits in place in the lab now, but for consistency in practice with keeping file sizes small, I think we will want to consider using Media for the purposes of leveraging the thumbnail service. This is non-blocking because I know the Media component needs some work. Let's add an issue to keep track of this and discuss it further?

srallen · 2019-09-18T15:11:09Z

packages/app-project/src/screens/ProjectHomePage/components/Hero/HeroContainer.spec.js

+      scope = nock('https://panoptes-staging.zooniverse.org/api')
+        .get('/translations')
+        .query(true)
+        .reply(200, {


Non-blocking, but if returning an error, just for consistency in mocking, this probably shouldn't be a 200 status.

Only the workflows endpoint is returning an error in this test, but both requests need to be successful in order to show the content (as currently written)

srallen · 2019-09-18T15:17:15Z

@beckyrother this will be ready for a design review when this merges and deploys to our staging instance (branch deployments are still WIP)

beckyrother · 2019-09-18T15:20:02Z

Sorry. Please tag me when it's ready.

srallen · 2019-09-18T15:41:30Z

@beckyrother no worries! I thought you might want to check it out as an actual app and not just screenshots.

srallen · 2019-09-18T16:19:46Z

I've opened #1133 to discuss the Media component use.

rogerhutchings added the enhancement New feature or request label Aug 14, 2019

rogerhutchings added this to the Project app (excluding classifier) milestone Aug 14, 2019

rogerhutchings requested a review from a team August 14, 2019 15:46

rogerhutchings added this to To do in Project app via automation Aug 14, 2019

srallen suggested changes Aug 14, 2019

View reviewed changes

rogerhutchings force-pushed the add-hero branch from 6a4f5fe to 3354016 Compare August 16, 2019 10:51

rogerhutchings requested a review from srallen August 16, 2019 10:52

srallen suggested changes Sep 3, 2019

View reviewed changes

This was referenced Sep 4, 2019

How to navigate to a specific workflow? Query param, or URL? #806

Closed

ADR-18 - Workflow routing #1107

Merged

Grommet theme needs republishing once #1077 merges #1111

Closed

rogerhutchings added 10 commits September 11, 2019 11:08

Add Hero widget

4c1a28d

- Set default screen size to 'small' - Update React to match other apps - Add Hero component - Add background - Add about link and intro - Add workflow loader

Don't read background out to screenreaders

6ab0da8

Improve propType checking

37b2b37

Use paragraph for intro text

562fd48

move error message to en.json

7a99fe0

test url construction in workflow select button

6c3bc4f

remove leftover debug

aa1bc6c

limit max-height of hero image

c2fa3b4

add tests

ef97a51

rogerhutchings force-pushed the add-hero branch from 8eac666 to ef97a51 Compare September 11, 2019 10:08

rogerhutchings and others added 5 commits September 12, 2019 10:01

Merge branch 'master' into add-hero

ffa5c41

Merge branch 'master' into add-hero

fe048d4

start async component tests

d36eef9

add remaining tests

890001d

rogerhutchings and others added 3 commits September 13, 2019 11:17

refactor tests

98d094a

Merge branch 'master' into add-hero

4385f4a

Merge branch 'master' into add-hero

127c62f

srallen approved these changes Sep 18, 2019

View reviewed changes

srallen and others added 6 commits September 18, 2019 11:19

Merge branch 'master' into add-hero

b8833e8

Merge branch 'master' into add-hero

70682ee

Merge branch 'master' into add-hero

8b01edf

Resize arrow

3e56221

Remove unused const

aa21f47

lint

a892d4e

rogerhutchings mentioned this pull request Sep 19, 2019

Inconsistent use of lib-react-component's Media component #1133

Closed

rogerhutchings modified the milestones: Shipping the project app, Shipping the new homepage Sep 19, 2019

Merge branch 'master' into add-hero

abcd798

eatyourgreens merged commit 175010f into master Sep 19, 2019

Project app automation moved this from To do to Done Sep 19, 2019

eatyourgreens deleted the add-hero branch September 19, 2019 16:16

This was referenced Sep 20, 2019

Add fallback for no workflows in Hero widget #1150

Closed

Add completeness bar to workflow selection buttons #1151

Closed

eatyourgreens mentioned this pull request Feb 22, 2021

Engaging Crowds: Subject set completeness #2016

Merged

18 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Hero widget #1077

Add Hero widget #1077

rogerhutchings commented Aug 14, 2019 •

edited

srallen left a comment

srallen Aug 14, 2019

rogerhutchings Aug 16, 2019 •

edited

srallen Sep 3, 2019

eatyourgreens Sep 4, 2019

rogerhutchings Sep 4, 2019

srallen Sep 4, 2019

rogerhutchings Sep 4, 2019

rogerhutchings Sep 5, 2019

eatyourgreens Sep 5, 2019

rogerhutchings commented Aug 16, 2019 •

edited

rogerhutchings commented Sep 2, 2019

srallen left a comment

srallen Sep 3, 2019

rogerhutchings commented Sep 13, 2019

rogerhutchings commented Sep 18, 2019

beckyrother commented Sep 18, 2019

srallen left a comment

srallen Sep 18, 2019

rogerhutchings Sep 19, 2019

srallen commented Sep 18, 2019

beckyrother commented Sep 18, 2019

srallen commented Sep 18, 2019

srallen commented Sep 18, 2019

Add Hero widget #1077

Add Hero widget #1077

Conversation

rogerhutchings commented Aug 14, 2019 • edited

Mobile

Desktop

Review Checklist

General

Apps

srallen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rogerhutchings Aug 16, 2019 • edited

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

rogerhutchings commented Aug 16, 2019 • edited

rogerhutchings commented Sep 2, 2019

srallen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rogerhutchings commented Sep 13, 2019

rogerhutchings commented Sep 18, 2019

beckyrother commented Sep 18, 2019

srallen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

srallen commented Sep 18, 2019

beckyrother commented Sep 18, 2019

srallen commented Sep 18, 2019

srallen commented Sep 18, 2019

rogerhutchings commented Aug 14, 2019 •

edited

rogerhutchings Aug 16, 2019 •

edited

rogerhutchings commented Aug 16, 2019 •

edited