Skip to content
This repository has been archived by the owner on Feb 28, 2024. It is now read-only.

BrowserStack Audit #222

Merged
merged 8 commits into from
Apr 25, 2018
Merged

BrowserStack Audit #222

merged 8 commits into from
Apr 25, 2018

Conversation

mateoclarke
Copy link
Contributor

@mateoclarke mateoclarke commented Apr 16, 2018 •
edited
Loading

Closes cityofaustin/techstack#146
Closes https://github.com/cityofaustin/coa-dev-internal/issues/21
Starts cityofaustin/techstack#308

  • Update the README
  • Add a section about browser compatibility
  • Polyfills for React Static in IE & Intl in IE and Safari < 10

There is definitely more comprehensive work we could be doing but I'm hesitant to over commit us this early in the project and without a dedicated QA resource. I really like this guideline cfpb has: https://github.com/cfpb/development/blob/master/guides/browser-support.md

Logged some more issues as a result of browsing that can be addressed in another PR.

@benweatherman benweatherman temporarily deployed to janis-staging-pr-222 April 16, 2018 22:16 Inactive
@mateoclarke mateoclarke temporarily deployed to janis-staging-pr-222 April 20, 2018 18:58 Inactive
@mateoclarke mateoclarke temporarily deployed to janis-staging-pr-222 April 21, 2018 20:58 Inactive
@mateoclarke mateoclarke removed the wip label Apr 21, 2018
@mateoclarke mateoclarke temporarily deployed to janis-staging-pr-222 April 22, 2018 16:41 Inactive
benweatherman
benweatherman approved these changes Apr 24, 2018
View reviewed changes
Copy link
Contributor

@benweatherman benweatherman left a comment

Choose a reason for hiding this comment

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

🐐

good addition of lots of background info to get people up and running

README.md Outdated

## Project Info

This work began under the he Austin Digital Services Discovery Project. Design, Technology, and Innovation Fellows worked in partnership between two City departments, Communications Public Information and Communications and Technology Management.
Copy link
Contributor

Choose a reason for hiding this comment

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

under the he Austin

README.md Outdated

### React-Static

This project uses [React-Static](https://github.com/nozzle/react-static) as a base framework for building static-progressive a React application/website. It's designed to with considerations for SEO, site performance, and user/developer experience.
Copy link
Contributor

Choose a reason for hiding this comment

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

for building static-progressive a React application/website

It's designed to with considerations

README.md Outdated

This project uses [React-Static](https://github.com/nozzle/react-static) as a base framework for building static-progressive a React application/website. It's designed to with considerations for SEO, site performance, and user/developer experience.

TODO: more on this decision + evaluation
Copy link
Contributor

Choose a reason for hiding this comment

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

meh

README.md Outdated

## Design assets
Since we use React-Static as our framework to render our React components as a static progressive website, its important for us to be able to test the final static build locally. In order to do this, we have two scripts.
Copy link
Contributor

Choose a reason for hiding this comment

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

its important

@mateoclarke mateoclarke temporarily deployed to janis-staging-pr-222 April 25, 2018 01:17 Inactive
ifsimicoded
ifsimicoded reviewed Apr 25, 2018
View reviewed changes
README.md Outdated

---
**To build and server**
Copy link
Contributor

@ifsimicoded ifsimicoded Apr 25, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Do you mean 'To Build and Serve' (no r)
Also I'm not sure we would ever run build.sh by itself -- but maybe we would?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point, we made the build script before the build and serve and I haven't used it since.

@ifsimicoded
Copy link
Contributor

ifsimicoded commented Apr 25, 2018 •
edited
Loading

Do we want to invest in making our serve-local build operable in IE so we can live debug in IE?
The app seems to die on the strip-ansi package that CRA has a fixed issue logged.
facebook/create-react-app#2691
It seems the upgraded version of strip-ansi uses arrow functions which isn't IE11 friendly. Tho our react app uses strip-ansi 3.0 not 4.0 (the culprit in the CRA issue above). 4.0 is a dependency of some other packages.

ifsimicoded
ifsimicoded approved these changes Apr 25, 2018
View reviewed changes
Copy link
Contributor

@ifsimicoded ifsimicoded left a comment

Choose a reason for hiding this comment

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

ieisslow

@mateoclarke
Copy link
Contributor Author

@ifsimicoded thanks for detail around how we would make serve-local work in IE. If its as easy as downgrading a dependency, I'd say why not give it a try and log it as an issue. But when it comes to live debugging in IE, wouldn't it be better to use serve-build since thats what a user would experience in production? But more is better, right?

@mateoclarke mateoclarke merged commit 6ffb89b into master Apr 25, 2018
@mateoclarke mateoclarke deleted the mc_browserstackin branch April 25, 2018 14:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@benweatherman benweatherman benweatherman approved these changes

@ifsimicoded ifsimicoded ifsimicoded approved these changes

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
@mateoclarke @ifsimicoded @benweatherman