Ux vote loading - Githubissues

ggggg commented 2 years ago

Added a loading wheel for when the user clicks on a vote until the server responds.
If the server doesn't respond than display error message (see image below)

Fixes: #73

Screenshots and screen captures:

Self-review checklist

Communicate decisions, questions, and potential concerns.

[ ] Explains differences from previous plans (e.g., issue description).
[ ] Highlights technical choices and bugs encountered.
[ ] Calls out remaining decisions and concerns.
[ ] Automated tests verify logic where appropriate.

Individual commits are ready for review:

[ ] Each commit is a coherent idea.
[ ] Commit message(s) explain reasoning and motivation for changes (what you are trying to accomplish? e.g. don't let user's age less than 0).

Completed manual review and testing of the following (please attach screenshots/gifs for UI changes and commit your test cases for functionality changes):

[ ] Visual appearance of the changes.
[ ] Responsiveness and internationalization.
[ ] Strings and tooltips.
[ ] End-to-end functionality of buttons, interactions and flows.
[ ] Corner cases, error conditions, and easily imagined bugs.

huonggiangbui commented 2 years ago

I also request another project owner to take a look, because I'm not sure if they want to add another library (Material UI)

ggggg commented 2 years ago

I cannot request anyone to review, only maintainers can. It was not my idea to add MUI, @huonggiangbui requested it in #73

huonggiangbui commented 2 years ago

LOL you can use any library you want, MUI is just a suggestion

huonggiangbui commented 2 years ago

MUI is fine imo, but I want to see what Shubh or Chris think about this

ggggg commented 2 years ago

LOL you can use any library you want, MUI is just a suggestion

Oh, sorry for the misunderstanding.

huonggiangbui commented 2 years ago

It's okay, I should make it more clear!

hiimchrislim commented 2 years ago

Personally, I wouldn't add in MUI. We already have Tailwind CSS and personally, I don't want to be adding in another dependency for this. For example: https://flowbite.com/docs/components/spinner/ - Just found this after a quick google search for a spinner using Tailwind

ggggg commented 2 years ago

Personally, I wouldn't add in MUI. We already have Tailwind CSS and personally, I don't want to be adding in another dependency for this. For example: https://flowbite.com/docs/components/spinner/ - Just found this after a quick google search for a spinner using Tailwind

The spinner wasn't what we added MUI for, we wanted a cleaner error message. But I'll come up with something.

ggggg commented 2 years ago

Used alert() because this message shouldn't pop up often and to avoid adding another library/component. Please review and let me know of any changes you want me to make.

logonoff commented 2 years ago

If tailwind is already a dependency a better solution that might look nicer is to use one of their modal components https://tailwindui.com/components/application-ui/overlays/modals

ggggg commented 2 years ago

If tailwind is already a dependency a better solution might look nicer to use one of their modal components https://tailwindui.com/components/application-ui/overlays/modals

I will look into it, thanks!

ggggg commented 1 year ago

@logonoff Sorry for this taking a while, I had midterms and was busy with other school stuff. Anyways, this should now use tailwind's alert box instead of the ugly javascript one. Here is a gif preview: gif_preview

embeddedt commented 1 year ago

I know this is incredibly picky, but I'm not liking that the spinner isn't centered around the C. :laughing: Is it easy to fix that?

The alert box looks great!

ggggg commented 1 year ago

Well the spinner is centered, I think anything else would look odd. A possible fix can be to mess around with the size of the spinner.

ggggg commented 1 year ago

Are any of these any better @embeddedt? chrome_spNEvwLG2X chrome_X0FsL5nITg

embeddedt commented 1 year ago

The issue is less obvious now but it's still not centered compared to the center of the C. I'm not sure why. It's weird, since I assume the buttons have centered text and the spinner is also centered.

embeddedt commented 1 year ago

Also, I like the first design in https://github.com/utmgdsc/PollVotingSystem/pull/74#issuecomment-1288165720 the best (the one with no background), as it looks cleaner IMO and should also be easier to port to a future dark mode.

ggggg commented 1 year ago

Hey is this still open?

utmgdsc / PollVotingSystem

Ux vote loading #74