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

Updated UI fixed layout of Blogs and Engage Components #395

Merged
merged 5 commits into from
Aug 20, 2023

Conversation

mayurjadhav2002
Copy link
Contributor

@mayurjadhav2002 mayurjadhav2002 commented Jul 20, 2023

Description

This PR fixes issue #392, the website UI now has fixed height and width for blog components as well as for the resources cards (Engage Component).

Updated UI

Blog Page
Resource Page

Before Updating

Blog Page

Resource page
Notes for Reviewers

Signed commits

  • Yes, I signed my commits.

@netlify
Copy link

netlify bot commented Jul 20, 2023

Deploy Preview for smp-spec ready!

Name Link
🔨 Latest commit d916953
🔍 Latest deploy log https://app.netlify.com/sites/smp-spec/deploys/64e0b5f520eb6200088f4d9f
😎 Deploy Preview https://deploy-preview-395--smp-spec.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@asubedy asubedy left a comment

Choose a reason for hiding this comment

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

Great work on the uniformity of cards @mayurjadhav2002, One small change, the hover effect on the post cards instead of the content being scaled up let's make the entire card pop up similar to Resources cards.

@mayurjadhav2002
Copy link
Contributor Author

mayurjadhav2002 commented Jul 24, 2023

Hey Hii, @asubedy I just completed the changes you mentioned. I wanted to ask you, should I remove that green eye on blog cards that can be seen on hover? because we are scaling the card itself.

@sudhanshutech
Copy link
Contributor

Let's discuss it on the websites call.
Please add this as an agenda item in the meeting minutes, if you would :)

https://docs.google.com/document/d/1XczAHXVe2FIWPqiF57ospJ43zw5cZQ7ui8mn39v5EvA/edit#heading=h.lohhtewfwima

@mayurjadhav2002
Copy link
Contributor Author

Let's discuss it on the websites call. Please add this as an agenda item in the meeting minutes, if you would :)

https://docs.google.com/document/d/1XczAHXVe2FIWPqiF57ospJ43zw5cZQ7ui8mn39v5EvA/edit#heading=h.lohhtewfwima

@sudhanshutech tried to open the link, but getting "Access Denied".

Copy link
Member

@asubedy asubedy left a comment

Choose a reason for hiding this comment

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

@mayurjadhav2002 looks great one small problem

Screen.Recording.2023-08-14.at.19.48.43.mov

In this recording you can see the text getting wrapped up onto next line while hovering. This doesn't look good. I guess only having the card popup on hover is good without the text getting wrap onto next line.

@mayurjadhav2002
Copy link
Contributor Author

@asubedy Hey, thanks for the review, I've applied the changes. also, I want to mention that I have found a new issue on the dashboard page the performance profile table which shows results is not responsive. take a look at the below screenshots.
On Desktop
scrnli_15_08_2023_00-58-57

On Mobile or Small screen devices
scrnli_15_08_2023_01-01-06

On mobile, it is just showing the profile and endpoints. It is not horizontally scrollable to see other information.
What's your view on this?
Can I work on this issue?
if yes then please let me know whether I should need to create a new issue or just solve it in this pr. I guess opening a new issue thread would be more convenient for other users. but please let me know your pov.

Thanks and Regards!

Copy link
Member

@asubedy asubedy left a comment

Choose a reason for hiding this comment

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

@mayurjadhav2002 The same text wrapping effect can be seen in the engage cards too, could you change that too.

@asubedy
Copy link
Member

asubedy commented Aug 16, 2023

@mayurjadhav2002 regarding the dashboard change .YES! that needs a fix you can go ahead and make issue for this and I will assign it to you.

@mayurjadhav2002
Copy link
Contributor Author

@asubedy I have removed the text wrapping effect from engage cards. please let me know if there are any more changes required. in the meantime, I will open a new issue regarding the dashboard page.

Copy link
Member

@asubedy asubedy left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you @mayurjadhav2002 🚀

@asubedy asubedy added this pull request to the merge queue Aug 20, 2023
Merged via the queue into service-mesh-performance:master with commit 64c7f15 Aug 20, 2023
7 of 11 checks passed
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.

None yet

3 participants