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.

Sign up for GitHub

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

Jump to bottom

Tag static files from a theme-gem with a sticker #406

Merged
merged 4 commits into from
Jun 30, 2017
Merged

Tag static files from a theme-gem with a sticker #406

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c580329
tag static files from a theme-gem with a sticker
ashmaroli Jun 23, 2017
e6c917b
move conditions to outside of return function
ashmaroli Jun 23, 2017
f9ef309
test rendering theme-indicator
ashmaroli Jun 25, 2017
327ea95
Merge branch 'master' into static-gem-sticker
mertkahyaoglu Jun 30, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions lib/jekyll-admin/apiable.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ def to_api(include_content: false)
output["name"] = basename
end

if is_a?(Jekyll::StaticFile)
output["from_theme"] = from_theme_gem?
end

output
end

Expand All @@ -61,6 +65,10 @@ def file_path
end
end

def from_theme_gem?
@base == site.in_theme_dir
end

# StaticFiles don't have `attr_accesor` set for @site or @name
def site
@site
Expand Down
11 changes: 10 additions & 1 deletion src/components/FilePreview.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,18 @@ export default class FilePreview extends Component {
}
return (
<div className="file-preview">
{
Copy link
Member

@mertkahyaoglu mertkahyaoglu Jun 23, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I know I've put a few inline conditions in the front-end but it's better to pull them out of return statement which makes it more readable.

let theme_indicator;
let btn_delete;

if(file.from_theme) {
  theme_indicator = <div...
} else {
  btn_delete = <button...
}

file.from_theme &&
<span className="theme-indicator">
<i className="fa fa-diamond" aria-hidden="true" title="Theme Asset" />
</span>
}
{nodeLink}
<span className="filename">{file.path}</span>
<button onClick={() => this.handleClickDelete(file.path)} className="delete" title="Delete file">x</button>
{
!file.from_theme &&
<button onClick={() => this.handleClickDelete(file.path)} className="delete" title="Delete file">x</button>
Copy link
Member

Choose a reason for hiding this comment

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

Why should we not allow deleting theme files?

Copy link
Member Author

Choose a reason for hiding this comment

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

Why should we not allow deleting theme files?

While I was greatly surprised that you asked this question, on reflecting further, I'm assuming you were talking about removing the Jekyll::StaticFile object from the site_payload rather than deleting the file proper..

To answer otherwise, three things to consider:

  • The theme-gem is supposed to be a read-only object for Jekyll.
  • We have no write-access outside site.source.
  • When users click delete, on a theme-file they're sending a delete request to the relative_path of the static file which is sanitized to point to a non-existing entity in the source_dir. (e.g. <theme>/assets/logo.png will resolve as <site.source>/assets/logo.png which doesn't exist. Otherwise, Jekyll would've read that file in to memory instead)

There's no valid use-case to allow write-access to the theme-gem in future either

}
</div>
);
}
Expand Down
15 changes: 15 additions & 0 deletions src/styles/staticfiles.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,21 @@
display: inline-table;
}
}
span.theme-indicator {
position: absolute;
bottom: 40px;
left: 0;
width: 32px;
height: 32px;
text-align: left;
background: $dark-red;
i {
margin: 0;
padding: 8px 7px 7px;
color: white;
font-size: 16px;
}
}
span.filename {
font-size: 14px;
line-height: 2;
Expand Down