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

Add cloud icons for remote favorite paths #3076

Merged
merged 3 commits into from
Sep 28, 2023

Conversation

robinkar
Copy link
Contributor

@robinkar robinkar commented Sep 27, 2023

This PR adds cloud icons for favorite paths for remote storages both in the navbar and in the file browser.

remote-storage-icons
The navbar items are currently somewhat ugly due to the spacing, but that is fixed in my other open PR (#3075).

<li class="nav-item"><%= link_to p.title || p.path.to_s, files_path(p.filesystem, p.path.to_s), class: "nav-link d bg-light" %>
<li class="nav-item">
<%= link_to files_path(p.filesystem, p.path.to_s), class: "nav-link d bg-light" do %>
<%= fa_icon "cloud", classes: '' %>
Copy link
Contributor

Choose a reason for hiding this comment

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

I've got to look this over - it looks like you've hard coded cloud icons for all favorite paths, so I'm either misreading it or something.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, let me double check that. Seems like some code may have gone missing.

Copy link
Contributor Author

@robinkar robinkar Sep 27, 2023

Choose a reason for hiding this comment

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

It had indeed gone missing. Added it now.

@robinkar robinkar force-pushed the remote-storage-icons branch from 3d41c92 to 5140353 Compare September 27, 2023 14:25
<li class="nav-item"><%= link_to p.title || p.path.to_s, files_path(p.filesystem, p.path.to_s), class: "nav-link d bg-light" %>
<li class="nav-item">
<%= link_to files_path(p.filesystem, p.path.to_s), class: "nav-link d bg-light" do %>
<%= fa_icon "cloud", classes: '' if p.remote? %>
Copy link
Contributor

Choose a reason for hiding this comment

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

I see what I'm missing - these links didn't have icons at all! Doesn't seem like we can use icon_uri here, but maybe a helper to determine the icon? That we we get folders and clouds.

image

Copy link
Contributor Author

@robinkar robinkar Sep 28, 2023

Choose a reason for hiding this comment

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

Added icons for the other favorite paths, and also Home Directory for consistency. Although I wonder if we could rely more directly on the OodAppLinks here.
image

Use folder icon for normal favorite paths, which previously had no icon.
Copy link
Contributor

@johrstrom johrstrom left a comment

Choose a reason for hiding this comment

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

Thanks!

@johrstrom johrstrom closed this Sep 28, 2023
@johrstrom johrstrom reopened this Sep 28, 2023
@johrstrom johrstrom closed this Sep 28, 2023
@johrstrom johrstrom reopened this Sep 28, 2023
@johrstrom johrstrom merged commit 430fc13 into OSC:master Sep 28, 2023
54 of 62 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants