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

New account settings design #2

Merged
merged 63 commits into from
Jun 29, 2023
Merged

New account settings design #2

merged 63 commits into from
Jun 29, 2023

Conversation

nsharma123
Copy link
Collaborator

No description provided.

jbuckner and others added 30 commits March 5, 2021 13:56
Add the base set of IA-customizations. Changes:

- Add .travis.yml file
- Change LICENSE to AGPL
- Add badges to Readme
- Set base font size
- Add circular dependency checking
- Export types
- Add sinon for test-helpers
- Update typescript
- Update some eslint settings
This makes it easier to create module demos since you can use Typescript all the way down.
* Add initial GitHub actions setup

* Remove Travis CI script and remove format script in GitHub Actions CI
This fixes an erroneous eslint typescript warning
Travis CI was removed in favor of GitHub Actions in #2.
@web/test-runner was using an older version of puppeteer, which wasn't working for M1 Macs. Now it's updated to a newer version.
Co-authored-by: Renovate Bot <bot@renovateapp.com>
Co-authored-by: Renovate Bot <bot@renovateapp.com>
Co-authored-by: Renovate Bot <bot@renovateapp.com>
Co-authored-by: Renovate Bot <bot@renovateapp.com>
Co-authored-by: Renovate Bot <bot@renovateapp.com>
Co-authored-by: Renovate Bot <bot@renovateapp.com>
* Add GH pages config + update Github actions build script
* Update README with details about build scripts and steps on setting up Github pages
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@github-actions
Copy link

github-actions bot commented Jun 6, 2023

PR Preview Action v1.4.4
Preview removed because the pull request was closed.
2023-06-29 21:08 UTC

Copy link
Contributor

@iisa iisa left a comment

Choose a reason for hiding this comment

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

hello, midway through code review - I've added some comments, suggestions, questions for discourse 🙂

q: can we make demo workable?

demo/index.html Outdated Show resolved Hide resolved
demo/index.html Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
src/models.ts Show resolved Hide resolved
src/ia-account-settings.interface.ts Show resolved Hide resolved
src/ia-account-settings.interface.ts Show resolved Hide resolved
src/ia-account-settings.interface.ts Outdated Show resolved Hide resolved
src/ia-account-settings.ts Outdated Show resolved Hide resolved
Copy link

@latonv latonv left a comment

Choose a reason for hiding this comment

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

Nice job on this! Lots of solid work.

I pointed out what seems like a blocker in the email availability condition which should be addressed. Otherwise, just some other suggestions & questions throughout to help improve code clarity and type accuracy.

demo/index.html Outdated Show resolved Hide resolved
src/ia-account-settings.ts Outdated Show resolved Hide resolved
src/ia-account-settings.ts Outdated Show resolved Hide resolved
src/ia-account-settings.ts Outdated Show resolved Hide resolved
src/ia-account-settings.ts Outdated Show resolved Hide resolved
src/services/backend-service.ts Outdated Show resolved Hide resolved
src/services/backend-service.ts Outdated Show resolved Hide resolved
src/services/util.ts Outdated Show resolved Hide resolved
test/ia-account-settings.test.ts Outdated Show resolved Hide resolved
test/ia-account-settings.test.ts Outdated Show resolved Hide resolved
Copy link

@latonv latonv left a comment

Choose a reason for hiding this comment

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

Thanks for the updates! Just had some more suggestions & questions on a few things.

src/components/authentication-template.ts Outdated Show resolved Hide resolved
src/ia-account-settings.ts Outdated Show resolved Hide resolved
src/ia-account-settings.ts Outdated Show resolved Hide resolved
src/ia-account-settings.ts Outdated Show resolved Hide resolved
src/ia-account-settings.ts Outdated Show resolved Hide resolved
src/ia-account-settings.ts Outdated Show resolved Hide resolved
src/ia-account-settings.ts Show resolved Hide resolved
src/services/util.ts Outdated Show resolved Hide resolved
Copy link

@latonv latonv left a comment

Choose a reason for hiding this comment

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

LGTM, just a few final small suggestions.

Also, just a reminder to add linting back to the test script before merge, as @iisa noted.

src/assets/eye.-open.ts Outdated Show resolved Hide resolved
src/ia-account-settings.ts Outdated Show resolved Hide resolved
src/ia-account-settings.ts Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@nsharma123 nsharma123 merged commit efc83e4 into main Jun 29, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants