-
Notifications
You must be signed in to change notification settings - Fork 50
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
Introduce "too late" presence option to event attendees #4528
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, love the look!
While the padding may feel like much - I currently like how it is evenly spaced vertically and horizontally. And anyways lets get it out there to get a feel of it
app/routes/events/components/EventAdministrate/AttendeeElements.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!
As for the design, it might also work well with a dropdown like in the bdb
app/routes/events/components/EventAdministrate/AttendeeElements.tsx
Outdated
Show resolved
Hide resolved
app/routes/events/components/EventAdministrate/AttendeeElements.tsx
Outdated
Show resolved
Hide resolved
I contemplated it. Even though it would save us space, it would require the user to click two times, instead of just one, so I ended up not doing it. |
b999072
to
46f30ef
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Neither ion-icons or font-awesome had a turtle icon, so I extracted one from Lucide (which we're planning on migrating to). I'm very open to design suggestions, as I'm not sure what I feel about this. The padding feels a bit much, so I might refactor the Icon component - but that will probably have to wait.
46f30ef
to
a4ddb84
Compare
Description
Related backend changes
Neither ion-icons or font-awesome had a turtle icon (which I thought was the most fitting), so I extracted one from Lucide (which we're planning on migrating to). I'm very open to design suggestions, as I'm not sure what I feel about this. The padding feels a bit much, so I might refactor the Icon component - but that will probably have to wait.
Result
If you've made visual changes, please check the boxes below and include images showing the changes. Descriptions are appreciated.
The automatic penalty:
Testing
Things work. Tested that all other icons that use the
TooltipIcon
component still work and look good.