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

Feat/dwa #196

Merged
merged 58 commits into from
Oct 9, 2024
Merged

Feat/dwa #196

merged 58 commits into from
Oct 9, 2024

Conversation

Johnnyevans32
Copy link
Contributor

@Johnnyevans32 Johnnyevans32 commented Oct 6, 2024

What type of PR is this? (check all applicable)

  • ♻️ Refactor
  • ✨ New Feature
  • 🐛 Bug Fix
  • 📝 Documentation Update
  • 👷 Example Application
  • 🧑‍💻 Code Snippet
  • 🎨 Design
  • 📖 Content
  • 🧪 Tests
  • 🔖 Release
  • 🚩 Other

Description

This PR adds Web5 connection converting it to a DWA.

Related Tickets & Documents

Resolves #66
Resolves #75

Mobile & Desktop Screenshots/Recordings

Screen.Recording.2024-10-06.at.14.mp4

Added code snippets?

  • 👍 yes
  • 🙅 no, because they aren't needed

Added tests?

  • 👍 yes
  • 🙅 no, because they aren't needed
  • 🙋 no, because I need help

No tests? Add a note

Added to documentation?

  • 📜 readme
  • 📜 contributing.md
  • 📓 general documentation
  • 🙅 no documentation needed

No docs? Add a note

[optional] Are there any post-deployment tasks we need to perform?

[optional] What gif best describes this PR or how it makes you feel?

thanos-thano

Copy link
Contributor

@blackgirlbytes blackgirlbytes left a comment

Choose a reason for hiding this comment

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

LOL at the gif.

I dont feel comfortable reviewing this so I will suggest @leordev and @acekyd review this PR please and thank you!

@acekyd
Copy link
Contributor

acekyd commented Oct 7, 2024

Checking this out. Thanks for the contribution @Johnnyevans32

export function useWeb5() {
const { web5 } = storeToRefs(useWeb5Store())

const protocol = 'https://didcomm.org/dwa-starter-vue'
Copy link
Member

Choose a reason for hiding this comment

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

since they are both framework agnostic, can you please maintain the original files below and connect with this composable?

this will make things simpler to maintain across all the dwa-starter examples

Copy link
Contributor Author

@Johnnyevans32 Johnnyevans32 Oct 7, 2024

Choose a reason for hiding this comment

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

done with this @leordev, I found the react implementation of the web5 composable a bit confusing hence why I previously opted for a different version

Copy link
Member

Choose a reason for hiding this comment

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

What I suggested was to copy these two files to your project and use them in the useWeb5 or in the best way you can think of. Is it possible?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah ok got that now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

should be ok now i hope @leordev

await page.goto('/settings')
await expect(page.locator('h1')).toHaveText('Settings')
})
// TODO: mock web5 connection for access to settings page
Copy link
Member

Choose a reason for hiding this comment

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

do you need help with this test or are you planning to add it back in one of the other projects tasks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah I do actually, i was going to get more info on this after finishing up the main pages components

Copy link
Member

Choose a reason for hiding this comment

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

lets do this... hold off on this test for now, and lets merge this pr as soon as you address my points above. then after that I can work on mocking/setting up the connection in playwright while you work on your next task. this way we dont block you!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

gotcha thanks!

@Johnnyevans32
Copy link
Contributor Author

hi @leordev, have you had the time to look at the updates I made, if there are additional changes I need to make.

@blackgirlbytes
Copy link
Contributor

@Johnnyevans32 i tested and this is ready to go..the only thing that needs to be fixed before I approve and merge is the conflict on the pnpm lock file. Thank you!

@Johnnyevans32
Copy link
Contributor Author

@Johnnyevans32 i tested and this is ready to go..the only thing that needs to be fixed before I approve and merge is the conflict on the pnpm lock file. Thank you!

sorted now @blackgirlbytes

Copy link
Contributor

@blackgirlbytes blackgirlbytes 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 contribution! This was a tough one.

@blackgirlbytes blackgirlbytes merged commit bc54787 into TBD54566975:main Oct 9, 2024
21 checks passed
@blackgirlbytes
Copy link
Contributor

Hey @taniashiba ..after we remove the leaderboard action from github..we need to manually add ANOTHER 10 points for @Johnnyevans32 to the leaderboard because this PR he did two issues in one..a large and a medium. So instead of having 115..he would have 115 + 10 (from other PR) + 10 (from this PR) = 135

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.

Create sidebar component - Vue Convert Vue PWA (Progressive Web App) to a DWA (Decentralized Web App)
4 participants