-
Notifications
You must be signed in to change notification settings - Fork 14
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
Calendar #23
Calendar #23
Conversation
… search. Otherwise it scans from the first element to find the match.
const openCalendar = () => { | ||
handleShowCalendar(); | ||
}; | ||
|
||
const closeCalendar = () => { | ||
handleCloseCalendar(); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two functions seem a bit unnecessary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True, I agree
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leaving the rest to our protagonist Kirk,
if (!isNumeric(timestamp)) { | ||
throw (new Error("Invalid timestamp provided.")); | ||
} | ||
this._logFile.state.logEventIdx = this._logFile.getLogEventIdxFromTimestamp(timestamp); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the newest private class variable syntax is using #. I'm not sure if the _ prefix here means something should be private.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes/Private_class_fields
* @param {number} timestamp | ||
*/ | ||
changeEventWithTimestamp (timestamp) { | ||
if (!isNumeric(timestamp)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't wanna be that guy, but
false === isNumeric(timestamp)
@@ -51,8 +67,14 @@ export function MenuBar ({ | |||
const {theme, switchTheme} = useContext(ThemeContext); | |||
|
|||
const [eventsPerPage, setEventsPerPage] = useState(logFileState.pages); | |||
const [calendarDateTime, setCalendarDateTime] = useState( | |||
new Date(Math.floor(Date.now() / 1000) * 1000) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could make the millisecond truncation into a function.
|
||
const handleCalendarChange = (newDate) => { | ||
if (newDate instanceof Date) { | ||
setCalendarDateTime(newDate); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use the millisecond truncation function here as well for consistent calendarDateTime state behavior (although maybe you already pass in a date value with the millisecond part removed).
Add Time Zone Toggle between Local and UTC.
Since the UI has been rewritten, can we close this PR and open a feature request for this feature? |
Sure |
Description
Add a new functionality in the menu bar called "Calendar" which allows users to select a timestamp (in UTC seconds). The viewer will then jump to the first event with the timestamp greater or equal to the selected one.
The UI looks like the following:
Validation performed