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

profile details #376

Merged
merged 22 commits into from
Oct 21, 2024
Merged

profile details #376

merged 22 commits into from
Oct 21, 2024

Conversation

vishalvivekm
Copy link
Contributor

Notes for Reviewers

This PR adds
image

Signed commits

  • Yes, I signed my commits.

Copy link

netlify bot commented Sep 24, 2024

Deploy Preview for bejewelled-pegasus-b0ce81 ready!

Name Link
🔨 Latest commit 12ad300
🔍 Latest deploy log https://app.netlify.com/sites/bejewelled-pegasus-b0ce81/deploys/67137aa1fddfc20008d36906
😎 Deploy Preview https://deploy-preview-376--bejewelled-pegasus-b0ce81.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

@leecalcote leecalcote left a comment

Choose a reason for hiding this comment

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

Whoa, this is not good. We've just leaked an API token. Why is a token needed here?

This specific API token is now leaked to the world and needs to be deleted from whatever account created it, @vishalvivekm

@vishalvivekm vishalvivekm marked this pull request as draft September 26, 2024 22:20
vishalvivekm and others added 7 commits September 27, 2024 03:51
Co-authored-by: Hortison <[email protected]>
Signed-off-by: Vivek Vishal <[email protected]>
Co-authored-by: Hortison <[email protected]>
Signed-off-by: Vivek Vishal <[email protected]>
Signed-off-by: GitHub <[email protected]>
Signed-off-by: GitHub <[email protected]>
Signed-off-by: GitHub <[email protected]>
@leecalcote
Copy link
Member

@vishalvivekm is this still "Draft"?

@leecalcote
Copy link
Member

@vishalvivekm is this still "Draft"?

@vishalvivekm is this still "Draft"?

@vishalvivekm vishalvivekm marked this pull request as ready for review October 8, 2024 07:48
@vishalvivekm
Copy link
Contributor Author

@vishalvivekm is this still "Draft"?

@vishalvivekm is this still "Draft"?

Apologies for missing the messages, no it's ready for review.

@@ -6,7 +6,7 @@
<a class="navbar-brand" href="/">
<span class="navbar-brand__logo navbar-logo">
{{ $svg := resources.Get "icons/logo.svg" }}
<img class="footer-logo" src="{{ $svg.Permalink }}" alt="logo" />
<img class="footer-logo" src="/images/logo.svg" alt="logo" />
Copy link
Member

Choose a reason for hiding this comment

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

Why was this changed? What does it have to do with the avatar?

Copy link
Member

Choose a reason for hiding this comment

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

Part of the reason that I’m asking is because I see that there is a footer logo class being used. I assume that this is the footer. Maybe the footer class needs to be renamed.

<div class="logo-container">
<img
src="/images/logos/meshery-light-icon.svg"
alt="Layer5 Meshery Logo"
Copy link
Member

Choose a reason for hiding this comment

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

There isn’t a Layer5 Meshery logo, just a Meshery logo.

}

function getCookieValue(cookieName) {
const cookies = document.cookie.split(";");
Copy link
Member

@leecalcote leecalcote Oct 8, 2024

Choose a reason for hiding this comment

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

This sort of string parsing is never ideal. Is this what was done on the other sites, too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah this from layer5.io and meshery.layer5.io

Signed-off-by: GitHub <[email protected]>
@leecalcote
Copy link
Member

Let's get a "Sign In" button presented in the corner when an anonymous visitor opens the site. Do you agree?

Screenshot 2024-10-09 at 3 49 02 PM

@leecalcote
Copy link
Member

Also, will you please put Kanvas and Cloud docs links on the left with Products on the right with avatar, like this:

Screenshot 2024-10-09 at 3 51 47 PM

@vishalvivekm vishalvivekm mentioned this pull request Oct 14, 2024
1 task
Signed-off-by: GitHub <[email protected]>
Signed-off-by: Vivek Vishal <[email protected]>
@layer5io layer5io deleted a comment from Touriist Oct 15, 2024
@vishalvivekm
Copy link
Contributor Author

image

Signed-off-by: GitHub <[email protected]>
@vishalvivekm
Copy link
Contributor Author

image

if(e.target != dropdown && dropdown.classList.contains("current")){
dropdown.classList.toggle("current");
document.querySelector(".dropdown-menu").classList.toggle("show");
function setupDropdown(dropdownId) {
Copy link
Member

Choose a reason for hiding this comment

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

A comment describing the function, would be great.

Copy link
Member

@leecalcote leecalcote left a comment

Choose a reason for hiding this comment

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

@vivek Vishal, the following pertain to both Safari and Chrome:

  1. the app grid menu is bouncing around.
  2. in this recording the cursor style is not consistently "pointer", although in other tests is seems fine.
  3. the app grid should darken in color on hover.
Screen.Recording.2024-10-16.at.10.53.45.AM.mov

@leecalcote
Copy link
Member

Each app button should be of equal size.

@leecalcote
Copy link
Member

Is this javascript really needed? CSS ::active doesn't cut it?

@leecalcote leecalcote merged commit 6558b9a into layer5io:master Oct 21, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants