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

Small navigation improvements #238

Merged
merged 7 commits into from
Dec 10, 2021

Conversation

Prasantacharya
Copy link
Contributor

Resolves #198
For non-logged in users, the "join the conversation" button sits at the bottom, but for admins, it is in its usual spot
Also the text now aligns with the paragraph instead of the icon

@mlx-bot
Copy link
Collaborator

mlx-bot commented Oct 16, 2021

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Prasantacharya
To complete the pull request process, please assign yhwang after the PR has been reviewed.
You can assign the PR to them by writing /assign @yhwang in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ckadner
Copy link
Member

ckadner commented Oct 19, 2021

Thank you @Prasantacharya -- this is better :-)

Do you think it could be even more improved by vertically centering the speech bubble icon?

Screen Shot 2021-10-19 at 9 26 05 AM

@Prasantacharya
Copy link
Contributor Author

@ckadner ive shifted down the icons a bit. Altho when i built the site to test, I didnt have the white pullet points, which I assume is what I would be centering with?

@ckadner ckadner requested review from ckadner and removed request for Tomcli October 19, 2021 23:36
@ckadner
Copy link
Member

ckadner commented Oct 19, 2021

The speech bubble icon is aligned nicely now, but now the bullet point is sitting too low.

Screen Shot 2021-10-19 at 4 32 32 PM

@ckadner
Copy link
Member

ckadner commented Oct 19, 2021

@Prasantacharya -- could you sign your commits to get the DCO check to pass?

@ckadner ckadner added the RCOS Potential work items for RCOS student interns label Dec 1, 2021
@ckadner
Copy link
Member

ckadner commented Dec 2, 2021

@Prasantacharya -- did you ever get a chance to find out why the bullet point shows up out of line? I screenshoted that on Firefox

Copy link
Member

@ckadner ckadner left a comment

Choose a reason for hiding this comment

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

@Prasantacharya -- if you can find out how to vertically align (center) the icon and text with the bullet point that would be great.

@Prasantacharya
Copy link
Contributor Author

image
this is the closest ive gotten it to, does this look right?

@ckadner
Copy link
Member

ckadner commented Dec 9, 2021

image this is the closest ive gotten it to, does this look right?

Yeah, did this require further code changes? Did you push those to this PR?

@@ -13,6 +13,9 @@ Allows upload, registration, execution, and deployment of:
- Datasets
- Notebooks

For more details about the project please follow this [announcement blog post](https://lfaidata.foundation/blog/2021/09/28/machine-learning-exchange-mlx/).
Copy link
Member

Choose a reason for hiding this comment

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

This change here seems unrelated. Could you rebase your PR or merge in the latest changes from main?

@ckadner ckadner merged commit 49a7052 into machine-learning-exchange:main Dec 10, 2021
Copy link
Member

@ckadner ckadner left a comment

Choose a reason for hiding this comment

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

/lgtm

Thanks @Prasantacharya

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RCOS Potential work items for RCOS student interns
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Small navigation improvements
4 participants