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.

Sign up for GitHub

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

Jump to bottom

feat(react): add FormSG icons, title and description #2042

Merged
merged 6 commits into from
Jun 11, 2021
Merged

feat(react): add FormSG icons, title and description #2042

merged 6 commits into from
Jun 11, 2021

Conversation

chowyiyin
Copy link
Contributor

@chowyiyin chowyiyin commented Jun 1, 2021 •
edited
Loading

This PR replaces the default icons, title and description of the React base app with those for FormSG.

Closes #1901

Solution

Files added:

  • favicon.ico: 32x32 and 16x16
  • favicon.svg
  • logo192.png: 192x192
  • logo512.png: 512x512
  • apple-touch-icon.png: 180x180
    Used Squoosh and SVGO to optimize svg and pngs as suggested in issue's linked article

Screenshots

On desktop browser
Screenshot 2021-06-03 at 11 34 36 AM

On iPhone
photo6179124987709992202
photo6179124987709992201

On Android device
Note: logo192.png used for homescreen; logo512.png and name in manifest.json used for splash screen.
Screenshot 2021-06-03 at 11 39 02 AM
Screenshot 2021-06-03 at 11 37 50 AM
Screenshot 2021-06-03 at 11 50 09 AM

Manual Tests

  • Open FormSG on different desktop browsers. Should have FormSG icon displayed on tab.
  • Open FormSG on other Apple and Android devices. Add to home screen. FormSG icon should be displayed on web clip, on app in home screen and splash screen (for Android only)

@chowyiyin chowyiyin changed the title feat: add FormSG icons, title and description feat(react): add FormSG icons, title and description Jun 1, 2021
@chowyiyin chowyiyin marked this pull request as ready for review June 1, 2021 09:00
karrui
karrui requested changes Jun 1, 2021
View reviewed changes
frontend/public/index.html Outdated Show resolved Hide resolved
frontend/public/index.html Show resolved Hide resolved
karrui
karrui reviewed Jun 7, 2021
View reviewed changes
Copy link
Contributor

@karrui karrui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the favicon.ico does not seem to be transparent and have a white background instead, should we revert it to follow favicon.svg? Maybe use https://realfavicongenerator.net/ for generating the necessary files

@karrui
Copy link
Contributor

karrui commented Jun 8, 2021

Closing this to reduce clutter. @chowyiyin pls reopen when you pick this up again.

@karrui karrui closed this Jun 8, 2021
@karrui
Copy link
Contributor

karrui commented Jun 9, 2021

Reopening this, React issues are a go!

@karrui karrui reopened this Jun 9, 2021
@karrui karrui force-pushed the form-v2/develop branch from 95a9a35 to eceb9c9 Compare June 9, 2021 07:55
karrui and others added 5 commits June 10, 2021 16:55
@karrui @chowyiyin
feat(react): add base react app directory in root of the application (#…
88e54a0 
…1819)

* chore: add initial packages and lint tooling

* feat(design-system): add base colours to theme

* feat(design-system): add base typography to theme

* feat(design-system): add Button styling variants

* feat(design-system): add initial Input outline styling

* feat(design-system): extend theme to use new colours/text/components

* chore(github-actions): add Chromatic CI/CD

* feat(Spinner): add Spinner custom component

* feat(Button): add Button component and story

* chore(storybook): set up storybook

* chore: update chromatic to only run on form-v2 branches

* chore: update chromatic action with correct working directory

* chore: update root ts-config to ignore src/frontend

* chore: add lint scripts to react frontend directory

* build: move React app to root, skip preflight checks

preflight checks due to conflicting node modules, but can ignore for now

* fix: update chromatic github action to the correct directory

* chore: use eslint's sort import instead of prettier's and update lint

prettier config is not very customization

* feat(storybook): add withPerformance addon

* fix: correct lint:fix script, set frontend eslint as root

setting eslint as root means it will not converge with root's eslint, preventing conflicts

* feat: add better stories for Buttons

* chore: update storybook addons to latest for perf

* fix: add correct working directory for Chromatic action

* fix: remove deleted storybook addon

* fix: add all missing node_modules in frontend

didn't get any errors locally due to having a root node_module.. which is not always going to be possible

* fix: update lint-staged to account for multi-folder repo

* fix: lint-staged not working for multi-repo

* ref: remove usage of FC, indexes now default export for code split

* feat: add aliases to frontend configuration

* refactor: make export default a one-liner

* refactor: use alias instead of aliasDangerous in cra rewire

* chore: delete react app readme
@karrui @chowyiyin
feat(react): add base react app directory in root of the application (#…
a6249e8 
…1819)

* chore: add initial packages and lint tooling

* feat(design-system): add base colours to theme

* feat(design-system): add base typography to theme

* feat(design-system): add Button styling variants

* feat(design-system): add initial Input outline styling

* feat(design-system): extend theme to use new colours/text/components

* chore(github-actions): add Chromatic CI/CD

* feat(Spinner): add Spinner custom component

* feat(Button): add Button component and story

* chore(storybook): set up storybook

* chore: update chromatic to only run on form-v2 branches

* chore: update chromatic action with correct working directory

* chore: update root ts-config to ignore src/frontend

* chore: add lint scripts to react frontend directory

* build: move React app to root, skip preflight checks

preflight checks due to conflicting node modules, but can ignore for now

* fix: update chromatic github action to the correct directory

* chore: use eslint's sort import instead of prettier's and update lint

prettier config is not very customization

* feat(storybook): add withPerformance addon

* fix: correct lint:fix script, set frontend eslint as root

setting eslint as root means it will not converge with root's eslint, preventing conflicts

* feat: add better stories for Buttons

* chore: update storybook addons to latest for perf

* fix: add correct working directory for Chromatic action

* fix: remove deleted storybook addon

* fix: add all missing node_modules in frontend

didn't get any errors locally due to having a root node_module.. which is not always going to be possible

* fix: update lint-staged to account for multi-folder repo

* fix: lint-staged not working for multi-repo

* ref: remove usage of FC, indexes now default export for code split

* feat: add aliases to frontend configuration

* refactor: make export default a one-liner

* refactor: use alias instead of aliasDangerous in cra rewire

* chore: delete react app readme
@chowyiyin
feat: add FormSG icons, title and description
9307364
@chowyiyin
feat: arecreate icon files
efd9eb5
@chowyiyin
feat: regenerate icons
9eb783e
@chowyiyin chowyiyin force-pushed the feat/baseAppIconTitleDescription branch from 5a95b9f to 9eb783e Compare June 10, 2021 09:06
karrui
karrui reviewed Jun 10, 2021
View reviewed changes
Copy link
Contributor

@karrui karrui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

favicon.ico seems to be quite large, is there a way to minimize the size?

karrui
karrui approved these changes Jun 10, 2021
View reviewed changes
Copy link
Contributor

@karrui karrui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just realized our current logomark is 100kb... omg....

@karrui karrui merged commit c835993 into form-v2/develop Jun 11, 2021
@karrui karrui deleted the feat/baseAppIconTitleDescription branch June 11, 2021 04:04
@karrui karrui mentioned this pull request Jun 15, 2021
Closed
karrui added a commit that referenced this pull request Jun 24, 2021
@chowyiyin @karrui
feat(react): add FormSG icons, title and description (#2042)
39155a4 
* feat(react): add base react app directory in root of the application (#1819)

* chore: add initial packages and lint tooling

* feat(design-system): add base colours to theme

* feat(design-system): add base typography to theme

* feat(design-system): add Button styling variants

* feat(design-system): add initial Input outline styling

* feat(design-system): extend theme to use new colours/text/components

* chore(github-actions): add Chromatic CI/CD

* feat(Spinner): add Spinner custom component

* feat(Button): add Button component and story

* chore(storybook): set up storybook

* chore: update chromatic to only run on form-v2 branches

* chore: update chromatic action with correct working directory

* chore: update root ts-config to ignore src/frontend

* chore: add lint scripts to react frontend directory

* build: move React app to root, skip preflight checks

preflight checks due to conflicting node modules, but can ignore for now

* fix: update chromatic github action to the correct directory

* chore: use eslint's sort import instead of prettier's and update lint

prettier config is not very customization

* feat(storybook): add withPerformance addon

* fix: correct lint:fix script, set frontend eslint as root

setting eslint as root means it will not converge with root's eslint, preventing conflicts

* feat: add better stories for Buttons

* chore: update storybook addons to latest for perf

* fix: add correct working directory for Chromatic action

* fix: remove deleted storybook addon

* fix: add all missing node_modules in frontend

didn't get any errors locally due to having a root node_module.. which is not always going to be possible

* fix: update lint-staged to account for multi-folder repo

* fix: lint-staged not working for multi-repo

* ref: remove usage of FC, indexes now default export for code split

* feat: add aliases to frontend configuration

* refactor: make export default a one-liner

* refactor: use alias instead of aliasDangerous in cra rewire

* chore: delete react app readme

* feat(react): add base react app directory in root of the application (#1819)

* chore: add initial packages and lint tooling

* feat(design-system): add base colours to theme

* feat(design-system): add base typography to theme

* feat(design-system): add Button styling variants

* feat(design-system): add initial Input outline styling

* feat(design-system): extend theme to use new colours/text/components

* chore(github-actions): add Chromatic CI/CD

* feat(Spinner): add Spinner custom component

* feat(Button): add Button component and story

* chore(storybook): set up storybook

* chore: update chromatic to only run on form-v2 branches

* chore: update chromatic action with correct working directory

* chore: update root ts-config to ignore src/frontend

* chore: add lint scripts to react frontend directory

* build: move React app to root, skip preflight checks

preflight checks due to conflicting node modules, but can ignore for now

* fix: update chromatic github action to the correct directory

* chore: use eslint's sort import instead of prettier's and update lint

prettier config is not very customization

* feat(storybook): add withPerformance addon

* fix: correct lint:fix script, set frontend eslint as root

setting eslint as root means it will not converge with root's eslint, preventing conflicts

* feat: add better stories for Buttons

* chore: update storybook addons to latest for perf

* fix: add correct working directory for Chromatic action

* fix: remove deleted storybook addon

* fix: add all missing node_modules in frontend

didn't get any errors locally due to having a root node_module.. which is not always going to be possible

* fix: update lint-staged to account for multi-folder repo

* fix: lint-staged not working for multi-repo

* ref: remove usage of FC, indexes now default export for code split

* feat: add aliases to frontend configuration

* refactor: make export default a one-liner

* refactor: use alias instead of aliasDangerous in cra rewire

* chore: delete react app readme

* feat: add FormSG icons, title and description

* feat: arecreate icon files

* feat: regenerate icons

* fix: fix unresolved merge conflicts from rebase

Co-authored-by: Kar Rui Lau <[email protected]>
nauynix
nauynix reviewed Aug 27, 2021
View reviewed changes
frontend/public/index.html
<link rel="icon" href="%PUBLIC_URL%/favicon.ico" />
<link rel="icon" type="image/svg+xml" href="%PUBLIC_URL%/icon.svg" />
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here it references icon.svg, but the file favicon.svg was added. Was it a typo?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, fixed in a later PR when I discovered it. May not have been merged yet though 😂

This was referenced Oct 4, 2022
Closed
Merged
@justynoh justynoh mentioned this pull request Oct 5, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@karrui karrui karrui approved these changes

@nauynix nauynix nauynix left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@chowyiyin @karrui @nauynix