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

Highlight new files #1610

Merged
merged 4 commits into from
Sep 18, 2017
Merged

Highlight new files #1610

merged 4 commits into from
Sep 18, 2017

Conversation

kwm4385
Copy link
Contributor

@kwm4385 kwm4385 commented Aug 15, 2017

Users want a way to determine if a file is currently being written to, especially in cases where downloading an incomplete file such as a heap dump would be undesirable.

image

The task file browser will highlight files that have been modified in the last 60 seconds.

Copy link

@andyhuang91 andyhuang91 left a comment

Choose a reason for hiding this comment

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

I really like the idea of highlighting files that are recently modified. Having the loading icon, row highlighting, and bolding the time might be a little much. Is bolding the time necessary?

@@ -40,6 +42,10 @@ function sortData(cellData, file) {
return file;
}

function isRecentlyModified(mtime) {
return new Date().getTime() / 1000 - mtime <= RECENTLY_MODIFIED_SECONDS;

Choose a reason for hiding this comment

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

Date.now() is equivalent to new Date().getTime()

@@ -64,18 +70,33 @@ function TaskFileBrowser (props) {
return _.sortBy(props.files, 'isDirectory').reverse();
}

function recentlyModifiedTooltip() {

Choose a reason for hiding this comment

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

This doesn't have to be a function. You can create it once and assign it to a variable.

const recentlyModifiedTooltip = <Tooltip />;

<OverlayTrigger overlay={recentlyModifiedTooltip} />

return (
<div>
<Breadcrumbs items={pathItems} />
<UITable
data={getFiles() || []}
keyGetter={(file) => file.name}
rowClassName={({mtime}) => { return isRecentlyModified(mtime) ? 'bg-info-light' : null; }}

Choose a reason for hiding this comment

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

This will only update if something else causes this component to re-render. Is this the behavior that you want?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The app globally refreshes this data every 60 seconds, and when the window regains focus, so this shouldn't get out of sync.

label=""
id="icon"
key="icon"
cellData={(file) => isRecentlyModified(file.mtime) &&

Choose a reason for hiding this comment

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

Because this depends on the current time, you should evaluate this once at the beginning of the render function instead of calling it 3 times.

@baconmania baconmania merged commit 1a1b256 into master Sep 18, 2017
@baconmania baconmania deleted the highlight-new-files branch September 18, 2017 19:34
@baconmania baconmania added this to the 0.17.0 milestone Sep 20, 2017
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.

4 participants