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

[Omn-192] - Settings View(Installation) #300

Merged
merged 16 commits into from Mar 29, 2022
Merged

[Omn-192] - Settings View(Installation) #300

merged 16 commits into from Mar 29, 2022

Conversation

gitstart-omnivore
Copy link
Contributor

@gitstart-omnivore gitstart-omnivore commented Mar 23, 2022

Spec Checklist

  • Does the figma design fit the ticket description ?
  • Are Api endpoints ready and properly documented ?
  • Added video describing your plan of action ?

Description

  • Omnivore has an installation page that helps users installed native apps (iOS and Mac) and browser extensions. The current version is available at /settings/installation. We have a temporary version of this page, but need to update to the new design / layout.
  • Added the tooltip and the select component.

House Keeping

  • Added video describing your plan of action ?
  • Added Loom video ?
  • Is Deployment Link passing ?
  • Have you assinged PR to yourself ?
  • Is Github action passing ?
  • Updated the appropriate tag ?

Ticket link (if applicable)

Task Link

How has this been tested?

I have checked it on multiple screen size

Types of changes

  • Bug fix
  • New feature
  • Refactor
  • Others

Checklist:

  • I have test my code on different browsers and devices to make sure it is cross-browser compatible (both desktop and mobile).
  • I have confirmed the payload for integrated API matches params on API docs
  • I have confirmed design matches Spec specified on Figma.

Language translation

  • Did you udpate/add a new text node ?
  • I have updated language mapping JSOn in locale

Remarks

  • I added a new package to achieve this task.
  • I have updated package.json

@vercel
Copy link

vercel bot commented Mar 23, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

omnivore-prod – ./

🔍 Inspect: https://vercel.com/omnivore/omnivore-prod/8DQ4xXDj1spe6CQ7m6oYojdiBpvW
✅ Preview: https://omnivore-prod-git-omn-192-omnivore.vercel.app

omnivore-demo – ./

🔍 Inspect: https://vercel.com/omnivore/omnivore-demo/GA9rZWsZdok3cGSCnpcnuPke1DLR
✅ Preview: https://omnivore-demo.vercel.app

@jacksonh
Copy link
Contributor

Looks like some missing properties:



14:10  Error: Unknown property 'clip-path' found, use 'clipPath' instead  react/no-unknown-property
--
11:37:05.090 | 51:17  Error: Unknown property 'stop-color' found, use 'stopColor' instead  react/no-unknown-property
11:37:05.090 | 52:28  Error: Unknown property 'stop-color' found, use 'stopColor' instead  react/no-unknown-property
11:37:05.090 | 62:31  Error: Unknown property 'stop-opacity' found, use 'stopOpacity' instead  react/no-unknown-property
11:37:05.090 | 63:31  Error: Unknown property 'stop-opacity' found, use 'stopOpacity' instead  react/no-unknown-property
11:37:05.090 | 74:17  Error: Unknown property 'stop-color' found, use 'stopColor' instead  react/no-unknown-property
11:37:05.090 | 75:31  Error: Unknown property 'stop-color' found, use 'stopColor' instead  react/no-unknown-property
11:37:05.090 | 76:31  Error: Unknown property 'stop-color' found, use 'stopColor' instead  react/no-unknown-property
11:37:05.090 | 77:28  Error: Unknown property 'stop-color' found, use 'stopColor' instead  react/no-unknown-property
11:37:05.090 | 87:31  Error: Unknown property 'stop-opacity' found, use 'stopOpacity' instead  react/no-unknown-property
11:37:05.090 | 88:31  Error: Unknown property 'stop-opacity' found, use 'stopOpacity' instead  react/no-unknown-property
11:37:05.091 | 99:17  Error: Unknown property 'stop-color' found, use 'stopColor' instead  react/no-unknown-property
11:37:05.091 | 100:31  Error: Unknown property 'stop-color' found, use 'stopColor' instead  react/no-unknown-property
11:37:05.091 | 101:31  Error: Unknown property 'stop-color' found, use 'stopColor' instead  react/no-unknown-property
11:37:05.091 | 102:31  Error: Unknown property 'stop-color' found, use 'stopColor' instead  react/no-unknown-property
11:37:05.091 | 103:31  Error: Unknown property 'stop-color' found, use 'stopColor' instead  react/no-unknown-property
11:37:05.091 | 113:17  Error: Unknown property 'stop-color' found, use 'stopColor' instead  react/no-unknown-property
11:37:05.091 | 114:28  Error: Unknown property 'stop-color' found, use 'stopColor' instead  react/no-unknown-property
11:37:05.091 | 114:49  Error: Unknown property 'stop-opacity' found, use 'stopOpacity' instead  react/no-unknown-property

@gitstart-omnivore
Copy link
Contributor Author

Looks like some missing properties:



14:10  Error: Unknown property 'clip-path' found, use 'clipPath' instead  react/no-unknown-property
--
11:37:05.090 | 51:17  Error: Unknown property 'stop-color' found, use 'stopColor' instead  react/no-unknown-property
11:37:05.090 | 52:28  Error: Unknown property 'stop-color' found, use 'stopColor' instead  react/no-unknown-property
11:37:05.090 | 62:31  Error: Unknown property 'stop-opacity' found, use 'stopOpacity' instead  react/no-unknown-property
11:37:05.090 | 63:31  Error: Unknown property 'stop-opacity' found, use 'stopOpacity' instead  react/no-unknown-property
11:37:05.090 | 74:17  Error: Unknown property 'stop-color' found, use 'stopColor' instead  react/no-unknown-property
11:37:05.090 | 75:31  Error: Unknown property 'stop-color' found, use 'stopColor' instead  react/no-unknown-property
11:37:05.090 | 76:31  Error: Unknown property 'stop-color' found, use 'stopColor' instead  react/no-unknown-property
11:37:05.090 | 77:28  Error: Unknown property 'stop-color' found, use 'stopColor' instead  react/no-unknown-property
11:37:05.090 | 87:31  Error: Unknown property 'stop-opacity' found, use 'stopOpacity' instead  react/no-unknown-property
11:37:05.090 | 88:31  Error: Unknown property 'stop-opacity' found, use 'stopOpacity' instead  react/no-unknown-property
11:37:05.091 | 99:17  Error: Unknown property 'stop-color' found, use 'stopColor' instead  react/no-unknown-property
11:37:05.091 | 100:31  Error: Unknown property 'stop-color' found, use 'stopColor' instead  react/no-unknown-property
11:37:05.091 | 101:31  Error: Unknown property 'stop-color' found, use 'stopColor' instead  react/no-unknown-property
11:37:05.091 | 102:31  Error: Unknown property 'stop-color' found, use 'stopColor' instead  react/no-unknown-property
11:37:05.091 | 103:31  Error: Unknown property 'stop-color' found, use 'stopColor' instead  react/no-unknown-property
11:37:05.091 | 113:17  Error: Unknown property 'stop-color' found, use 'stopColor' instead  react/no-unknown-property
11:37:05.091 | 114:28  Error: Unknown property 'stop-color' found, use 'stopColor' instead  react/no-unknown-property
11:37:05.091 | 114:49  Error: Unknown property 'stop-opacity' found, use 'stopOpacity' instead  react/no-unknown-property

Ohh! Fixing it now

@jacksonh
Copy link
Contributor

The dropdown selector shouldn't have this shadow on the main button, just on the dropdown when it opens.

Screen Shot 2022-03-23 at 21 59 53

@jacksonh
Copy link
Contributor

The Download button for the browser extensions should redirect the user to the appropriate page. You can see the URLs in the old code:

const extensionDownloadLinks = {
  chrome: 'https://omnivore.app/install/chrome',
  safari: 'https://omnivore.app/install/mac',
  edge: 'https://omnivore.app/install/edge',
  firefox: 'https://omnivore.app/install/firefox',
}

@jacksonh
Copy link
Contributor

The images (both preview images, and the Download on the AppStore) all look a little low resolution on my laptop. I think we need high definition versions of these exported from Figma also.

@jacksonh
Copy link
Contributor

For both the Learn more underlined text they should be links to /help/saving-links

@jacksonh
Copy link
Contributor

it looks like we need to break to the mobile design a little earlier or make this wide design more responsive

Screen Shot 2022-03-23 at 22 24 33

@jacksonh
Copy link
Contributor

These should act as a toggle, where you can only select one option (default to the first selected).

It doesn't actually matter, since no matter what the user has selected we will redirect to /install/apple and the Apple App Store shows the appropriate app.

Screen Shot 2022-03-23 at 22 29 36

@jacksonh
Copy link
Contributor

🚢

@jacksonh jacksonh deleted the OMN-192 branch March 29, 2022 00:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants