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

Improved Event List UI #117

Merged
merged 4 commits into from
Aug 24, 2022

Conversation

bogdankharchenko
Copy link
Collaborator

@bogdankharchenko bogdankharchenko commented Aug 18, 2022

👋

UI isn't really my thing - but I figured I can make some minor tweaks to events page.

I'm not married to the design, if you don't think its a good idea, feel free to modify or just close the pr.

Proposed Design

Screen Shot 2022-08-18 at 2 30 59 PM

Screen Shot 2022-08-18 at 2 31 19 PM

Original Design

Screen Shot 2022-08-18 at 12 04 12 AM

@allella
Copy link
Member

allella commented Aug 18, 2022

@bogdankharchenko Thanks, this certainly addresses a few things in the current UI that are not great.

For instance, now showing "AM/PM" instead of "24 hour" times is nice.

Overall, I think it's easier on the eyes.

  • We have the "Add to Calendar" feature that's not represented in the PR. Are you proposing removing that feature?
  • It would be nice to make the AM / PM all CAPS
  • I'd either remove the '22 year part or make it the full year, 2022

We've never put adequate thought into testing the site for mobile responsive, accessibility, performance. These are all areas where I'm sure we could do a better job. This PR could be a good opportunity to do a bit of extra testing and probing to cover these other areas.

@bogdankharchenko
Copy link
Collaborator Author

@allella I will push these suggestions.

The Add to Calendar does exist, if you click on the date, definitely not as obvious.

My next PR, I want to add a iCal feed which can just stream events to your iphone/google calendar instead of adding an individual event - this way you can always be informed without checking the site.

I think that would be a nice feature.

@allella
Copy link
Member

allella commented Aug 18, 2022

Alright.

We do have #109 still being an issue and I believe it's to do with the caching Zach put in place on the Laravel side because the API from which it sources the data doesn't have the duplication.

We'd want to get that buttoned up before publishing something to people's calendars and risking having bad info. I haven't gotten into the guts of the Laravel app and the caching, since Zach has mostly responded to those requests, but it would be great to have someone to supplement what he's been able to volunteer.

Also, I've been part of a related freeCodeCamp project, called Chapter, which we hope will offer a free self-hosted alternative to Meetup.com. One of the things that's part of the MVP for it is to use a Google Calendar API approach for updating individual events to which people have RSVPed. I'm not even clear how that API approach will work, but I did put other notes related to the more traditional iCal files that Meetup and Eventbrite send with RSVPs and it's very limited in that they will never update an event if something is changed about it.

I believe you're talking about a feed URL of all events, instead of tracking individual events. I know you can easily add a new toggle on / off calendar to apps like Google Calendar, and that it would refresh itself, but I'm not sure many consumers of the HackGreenville app will want to see dozens of random tech events on their personal calendar.

@bogdankharchenko
Copy link
Collaborator Author

bogdankharchenko commented Aug 18, 2022

Pushed new changes to UI, also added mobile screenshot - this is optimized to be responsive across all devices.

"Feature" Add to Calendar is only assessable via the "event date" on click event.

Copy link
Collaborator

@zach2825 zach2825 left a comment

Choose a reason for hiding this comment

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

Nice work! The events page looks a lot better! 🔥

@allella
Copy link
Member

allella commented Aug 18, 2022

Not sure if this is part of the free font-awesome icons, but perhaps there's something like https://fontawesome.com/icons/calendar-plus?s=solid with a mouseover / span tag that says "Add {Event Name} {Event Date / Time} to Google Calendar" so it works on mouse over and probably would be good for accessibility.

@allella
Copy link
Member

allella commented Aug 18, 2022

The calendar icons on the homepage have a mouseover text that's hidden, so we might be able to reuse that pattern for a calendar-plus font icon.

image

@bogdankharchenko
Copy link
Collaborator Author

@allella does this look any better?

Screen Shot 2022-08-18 at 8 43 46 PM

@allella
Copy link
Member

allella commented Aug 19, 2022

Also, just a reminder, since I just used it myself for the Slack app background color, that Zach's wife created this style guide
https://hackgreenville.com/styles

@bogdankharchenko
Copy link
Collaborator Author

@allella These are all the default bootstrap theme styles, which are represented in the project.

@allella
Copy link
Member

allella commented Aug 19, 2022

Looks good. Does it have a mouseover for text to well people and screenreaders what that icon does / means?

@allella
Copy link
Member

allella commented Aug 19, 2022

@all-contributors add @bogdankharchenko for code and design

@allcontributors
Copy link

@allella

I've put up a pull request to add @bogdankharchenko! 🎉

@bogdankharchenko
Copy link
Collaborator Author

@allella added screen reader info, and hover title.

@allella
Copy link
Member

allella commented Aug 20, 2022

I'm not sure of the best practices on this, but my guess is that since we'll have a lot of "Add to Calendar" icons, and other text for that matter, that we'll want to be more verbose in some places and less in others.

For instance, on a screen reader it may be good to have "Add {something unique about the event} to Calendar". Otherwise, it may be hard to differentiate what's about to be added. I know it's an edge case, but it's probably the sort of thing that doesn't hurt to be more verbose on a mouseover / tooltip and is more useful for the accessibility case.

@bogdankharchenko
Copy link
Collaborator Author

@allella just turn on screen reader in safari and see how it looks

@allella
Copy link
Member

allella commented Aug 22, 2022

I don't have my dev system setup because I just upgraded my OS and it blew away my local PHP and extensions.

If you're happy with it, then I'll give it another look when it makes it to the staging server.

@bogdankharchenko
Copy link
Collaborator Author

Got it. I think this is moving in the right direction. We can certainly iron this out as we get feedback in staging/prod.

I say merge and then we deal with improvements later

@allella allella merged commit 78ccece into hackgvl:develop Aug 24, 2022
@allella
Copy link
Member

allella commented Aug 24, 2022

This is merged and staged. Though, I need to do some server upgrades so I'll do that before pushing this live.

https://stage.hackgreenville.com/events

@allella
Copy link
Member

allella commented Aug 25, 2022

Thanks again. This is now live. And, the server is now using PHP 8.1, so now there are less hassle trying to stage and deploy things.

https://hackgreenville.com/events

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.

3 participants