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

Frontend #10

Open
wants to merge 170 commits into
base: master
Choose a base branch
from
Open

Frontend #10

wants to merge 170 commits into from

Conversation

@VaughnVernon
Copy link
Contributor

@donnisnoni Thanks! I have requested @Florian-Schoenherr to review your work.

Florian: please review if possible. @donnisnoni is a recent graduate and asked for an opportunity to learn Svelte and Sappier, so I gave him vlingo-auth UI to implement. If you can't review please reassign to Abdullah (it's just that he has other piorities). Thanks.

@itsdonnix
Copy link
Author

Please guide me @Florian-Schoenherr

@Florian-Schoenherr
Copy link

Hi @donnisnoni!
You didn't have to copy over utils or stores from schemata, haha.
Your sign-off is missing (relevant to DCO), you're using vscode so there are settings for that:
always sign off

Editorconfig/Prettier/eslint is a great addition, honestly would've used it myself if I understood better what/when these format stuff.

Regarding Svelte and Sapper, the Docs/Tutorial on their sites are pretty great and for some more specific questions there's also a Svelte Discord Server. Oh and I see you've build a little project with it before, so maybe only Sapper is new to you?

Smelte is not that good, SMUI is not maintained - we use svelte-materialify.

@Florian-Schoenherr
Copy link

@VaughnVernon in #5 there was some UI implemented, if @donnisnoni knows Vue/Vuetify he can take a look there, too, for some of the structure?

@VaughnVernon
Copy link
Contributor

VaughnVernon commented Feb 18, 2021

@Florian-Schoenherr yes I noted there is an existing UI and mockups in docs for @donnisnoni to use.

@abdullahcalisir12 said that the current UI code looked like an exact copy from schemata with no changes to reflect auth language and use cases. Of course that won't work 😃

@itsdonnix
Copy link
Author

Thank's @Florian-Schoenherr!

Yes, I never learn sapper before 😁 .

I've used vue & vuetify before. BTW, I use them in my thesis. I will take a look on #5.

@itsdonnix
Copy link
Author

You didn't have to copy over utils or stores from schemata, haha.

Hahaha.. I just try to read the code and hope that I can understand how it works... 😀

@itsdonnix
Copy link
Author

You guys can view the #5 version on http://tightfisted-sock.surge.sh/

@itsdonnix
Copy link
Author

Sorry,, the signing key is still using my old account 😁

@itsdonnix itsdonnix force-pushed the frontend branch 2 times, most recently from 4b2b232 to a6feef5 Compare February 19, 2021 08:57
@Florian-Schoenherr
Copy link

You guys can view the #5 version on http://tightfisted-sock.surge.sh/

That's the old version? That's pretty nice.
It uses a Datatable, which materialify doesn't have right now. I would either suggest building your own with materialify components or, probably easier for now, using a standalone svelte "table" or "datatable" component, until materialify has one.

@itsdonnix
Copy link
Author

itsdonnix commented Feb 19, 2021

It uses a Datatable, which materialify doesn't have right now.

Yeah... Unfortunately , I think of it from yesterday... Like ohhh,,, need more work 😁

I would either suggest building your own with materialify components or, probably easier for now, using a standalone svelte "table" or "datatable" component, until materialify has one.

OMG,, it will take times if i must create one, I would try to find existing svelte datatable lib instead 😃

@itsdonnix
Copy link
Author

itsdonnix commented Feb 19, 2021

That's the old version? That's pretty nice.

@Florian-Schoenherr Agree!! Therefore, I thought yesterday, "This is so good, why don't I continue working on it?"

But I'm thinking of getting some experience with svelte and sapper (although svelte-kit will replace that)

BTW, If you have feedback, please let me know 😄.

@itsdonnix itsdonnix force-pushed the frontend branch 8 times, most recently from 4dd62bd to dd75f89 Compare February 20, 2021 04:32
@itsdonnix
Copy link
Author

Take a look guys... 👀 @VaughnVernon @Florian-Schoenherr

@itsdonnix itsdonnix force-pushed the frontend branch 2 times, most recently from e832c17 to 92adaa0 Compare February 20, 2021 05:21
Copy link

@Florian-Schoenherr Florian-Schoenherr left a comment

Choose a reason for hiding this comment

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

All the copied schemata files make it really hard to review.
I have looked at the code, nice idea with the Vercel Preview, don't know if we need it but if you could put the "preview link" somewhere here, it would be nice for reviewing Design :)

Regarding schemata code: not much of it is reusable, I would suggest looking at xoom-starter code instead (not the AggregateDialog, but the normal forms). I just realized I never wrapped my forms in <form>, which I should've.

src/main/frontend/src/routes/index.svelte Show resolved Hide resolved
src/main/frontend/src/stores.js Outdated Show resolved Hide resolved
@itsdonnix
Copy link
Author

itsdonnix commented Feb 23, 2021

Guys.. really... I dont understand the wireframes... i think I will just design the UI without care about the interaction since it so hard to understand

so it consistent with the delete dialog

Signed-off-by: Don Alfons Nisnoni <[email protected]>
Signed-off-by: Don Alfons Nisnoni <[email protected]>
Signed-off-by: Don Alfons Nisnoni <[email protected]>
Signed-off-by: Don Alfons Nisnoni <[email protected]>
Signed-off-by: Don Alfons Nisnoni <[email protected]>
Add check command to scripts;
Run the check command before build

Signed-off-by: Don Alfons Nisnoni <[email protected]>
Signed-off-by: Don Alfons Nisnoni <[email protected]>
Signed-off-by: Don Alfons Nisnoni <[email protected]>
Signed-off-by: Don Alfons Nisnoni <[email protected]>
Signed-off-by: Don Alfons Nisnoni <[email protected]>
@itsdonnix itsdonnix marked this pull request as ready for review March 29, 2021 00:35
Signed-off-by: Don Alfons Nisnoni <[email protected]>
Signed-off-by: Don Alfons Nisnoni <[email protected]>
Signed-off-by: Don Alfons Nisnoni <[email protected]>
@VaughnVernon
Copy link
Contributor

@Florian-Schoenherr The other day @donnisnoni informed me that he thinks this UI has been completed. Would it be possible for you to review this in the next few days and see what you think? I needs to be tested with the server using HTTP requests. I am not sure how much of that's already been accomplished but given your experience with the other tool UIs this should be straightforward for you. Thanks!

@itsdonnix
Copy link
Author

Thanks @VaughnVernon ! I just remember that assign members/group role feature is not finished yet

@VaughnVernon
Copy link
Contributor

@donnisnoni What is the completion date for this?

@itsdonnix
Copy link
Author

@VaughnVernon Can I completed my work here?

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.

4 participants