Skip to content

Wins works#1132

Merged
ItsLaszlo merged 27 commits intohackforla:gh-pagesfrom
jbubar:wins-works
Mar 8, 2021
Merged

Wins works#1132
ItsLaszlo merged 27 commits intohackforla:gh-pagesfrom
jbubar:wins-works

Conversation

@jbubar
Copy link
Member

@jbubar jbubar commented Feb 28, 2021

This is a collaborative PR between me and @akibrhast. He redesigned the structure of the wins page, and I put the finishing touches. @akibrhast put most of the work in for this pr.
I also added some code for restructuring the card so that the quotes show up in the right place

Fixes #1066
closes #1060

@jbubar jbubar marked this pull request as draft February 28, 2021 17:17
@jbubar jbubar marked this pull request as ready for review February 28, 2021 17:53
@jbubar jbubar mentioned this pull request Feb 28, 2021
16 tasks
@jbubar
Copy link
Member Author

jbubar commented Feb 28, 2021

desktop view

screencapture-localhost-4000-wins-2021-02-28-09_59_21

Mobile

screencapture-localhost-4000-wins-2021-02-28-10_00_13

overlay

Screen Shot 2021-02-28 at 9 59 51 AM

@jbubar jbubar marked this pull request as draft February 28, 2021 20:45
Merge conflicts that slipped through the cracks
@jbubar
Copy link
Member Author

jbubar commented Mar 1, 2021

@martham0 Thanks for pointing that out. not sure how that slipped through the cracks. Next time I will be a lot more careful with resolving merge conflicts.
Does that sort it out?

@jbubar jbubar requested a review from ItsLaszlo March 1, 2021 22:52
Copy link
Member

@akibrhast akibrhast left a comment

Choose a reason for hiding this comment

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

Everything works as expected! Ready to merge once docker-compose.yml has been removed from the commit. @jbubar

Copy link
Member

@ItsLaszlo ItsLaszlo left a comment

Choose a reason for hiding this comment

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

@martham0 Thanks for pointing that out. not sure how that slipped through the cracks. Next time I will be a lot more careful with resolving merge conflicts.
Does that sort it out?

Yup! I am able to run it.

@ItsLaszlo
Copy link
Member

Everything seems to be working as expected! Only a quick note about a possible duplicate file mentioned above.

This might be nitpicky and maybe a cause for another issue to be created but when using the dropdown filters the cursor change isn't accurate. It changes to cursor: pointer when hovering over anywhere in the dropdown except for the checkbox. On the checkbox, it turns back into a normal cursor. This can be a bit confusing for the user and implies they can select the category by clicking it rather than clicking the checkbox which isn't the case.

@akibrhast
Copy link
Member

Everything seems to be working as expected! Only a quick note about a possible duplicate file mentioned above.

This might be nitpicky and maybe a cause for another issue to be created but when using the dropdown filters the cursor change isn't accurate. It changes to cursor: pointer when hovering over anywhere in the dropdown except for the checkbox. On the checkbox, it turns back into a normal cursor. This can be a bit confusing for the user and implies they can select the category by clicking it rather than clicking the checkbox which isn't the case.

@martham0 Could you please open a new ticket with what you mentioned here? The same behavior can also be seen on the projects page as well. I am guessing we could make a ticket that calls for an action to fix it in both places?

@ItsLaszlo
Copy link
Member

Everything seems to be working as expected! Only a quick note about a possible duplicate file mentioned above.
This might be nitpicky and maybe a cause for another issue to be created but when using the dropdown filters the cursor change isn't accurate. It changes to cursor: pointer when hovering over anywhere in the dropdown except for the checkbox. On the checkbox, it turns back into a normal cursor. This can be a bit confusing for the user and implies they can select the category by clicking it rather than clicking the checkbox which isn't the case.

@martham0 Could you please open a new ticket with what you mentioned here? The same behavior can also be seen on the projects page as well. I am guessing we could make a ticket that calls for an action to fix it in both places?

Will do! The issue can be found here: #1155

@akibrhast
Copy link
Member

Hey @jbubar If I am not mistaken.. This pull request will also close
#1060 .
Could you confirm that, and if it is case , link it to this pull request

@jbubar
Copy link
Member Author

jbubar commented Mar 6, 2021

Hey @jbubar If I am not mistaken.. This pull request will also close
#1060 .
Could you confirm that, and if it is case , link it to this pull request

@akibrhast. Right! I did. and I will create a new issue for when it is done #1163

Thanks. and thanks for your patience. been moving around a bunch this week. Next week I will have some more free time. Best of luck this sunday :))

@jbubar jbubar requested review from ItsLaszlo and akibrhast March 6, 2021 07:23
Commented out the link to the new  form, since it is still in development and we are still using the google form
Copy link
Member

@akibrhast akibrhast left a comment

Choose a reason for hiding this comment

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

Everything looks good and ready for merge. @martham0 Thanks for creating that issue, and feel free to go ahead and merge it once you have had the time to review it yourself :D!

@akibrhast akibrhast self-requested a review March 6, 2021 19:51
Copy link
Member

@drubgrubby drubgrubby left a comment

Choose a reason for hiding this comment

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

This is really great work! I love the new way of structuring complex pages.

The only thing I notice is a small thing on the layout of the cards: The linkedIn and GitHub images seem to be much closer together than they are on the current site.

Current site:
image

This PR:
image

This may be correct, but I didn't see any discussion of it so I'd like to have @daniellex0 sign off on it before we merge.

@jbubar
Copy link
Member Author

jbubar commented Mar 8, 2021

The only thing I notice is a small thing on the layout of the cards: The linkedIn and GitHub images seem to be much closer together than they are on the current site.

good catch!

@drubgrubby
Copy link
Member

@jbubar - Thanks for making the change to the space between the linkedin and github images. It looks good to me and ready to merge.

@martham0 - It seems like this is ready to merge. If you agree, then please go ahead and squash and merge.

@ItsLaszlo
Copy link
Member

Everything looks good to me. I'll squash and merge!

@ItsLaszlo ItsLaszlo merged commit 1df26a7 into hackforla:gh-pages Mar 8, 2021
akibrhast added a commit to akibrhast/website that referenced this pull request Mar 8, 2021
akibrhast added a commit that referenced this pull request Mar 8, 2021
@roslynwythe roslynwythe mentioned this pull request Apr 23, 2023
9 tasks
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.

Refactor the wins page code to be cleaner Create a wins form.

4 participants