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

Improving UI/UX of the Explorer page Fixes #1451 #1738

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

yashkumar2603
Copy link

@yashkumar2603 yashkumar2603 commented Oct 16, 2024

What does this change?

Fixes the issue #1451
This PR aims to solve the bland UI of the Explorer page, as mentioned in this issue. I have done this by restyling the page without using any extra dependencies or imports. I have added a few colors to the theme, which go well with the overall application color palette. Please let me know if they need to be changed. I am attaching an image of the new UI vs. the old UI of the explorer page below to give an idea of what the improvements look like -

New UI

image

Old UI -

image

How was this tested?

How have you tested your changes?
I have tested the UI via running a dev server.
And mobile and tablet responsiveness by testing on those devices. The new UI is responsive, smooth and works well even in low network speed.

Accessibility

If applicable to your changes, have you:

  • Tested with screen reader
  • Checked to see if navigable by keyboard
  • Ensured that the colour contrast is passing

Lastly, this is my first contribution to any codebase of this size; please let me know of any mistakes I made. I want to learn more from the fantastic people.

Copy link

vercel bot commented Oct 16, 2024

@yashkumar2603 is attempting to deploy a commit to the Public Convenience Ltd Team on Vercel.

A member of the Team first needs to authorize it.

Copy link

vercel bot commented Oct 16, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
toiletmap ✅ Ready (Inspect) Visit Preview Oct 17, 2024 9:41am

I noticed some issues with the responsiveness on mobile devices after the PR was deployed onto Vercel, I have fixed those.
The problems were -
- Table was sticking to the right side
- Page title and the navbar was sticking to the left side
Changes -
- Added Media Query based font size handling and handled horizontal overflow in table
@yashkumar2603
Copy link
Author

yashkumar2603 commented Oct 17, 2024

Hello Oliver, the PR I submitted yesterday had some issues with mobile responsiveness, as I saw after the Vercel preview deployment. I have fixed those in the same PR using as minimal haphazard CSS as possible, keeping the codebase clean. Please review the changes. Also, I need help understanding why the Cypress tests are failing; is it my fault?
I can see that the failures are related to Authentication. I did not touch the Authentication as it was not required to make the UI changes. But if there is something I can do to make all the tests pass and get this PR merged, please let me know.
Please forgive me for any silly mistakes I made; I am new to this and willing to learn more and make a meaningful impact in the process.

@ob6160
Copy link
Member

ob6160 commented Oct 17, 2024

Hi @yashkumar2603, firstly thank you for taking the time to contribute to our project!

Our Cypress end to end tests rely on having access to secrets, but because this is coming from a fork of our project these secrets are withheld from these runs of the tests for security reasons. So no, not your fault at all! As it's rare that we get external contributions like this we haven't looked at a way to make this work. We don't have end to end test coverage for the explorer pages, so in this case I think we'll be fine if they don't run here (although I'll run them locally as a check)

We'll review your pull request soon, hopefully within the next week. Thanks again for your contribution! :)

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.

2 participants