diff --git a/project/effect.cpp b/project/effect.cpp
index f32598be5..43d2e2ec2 100644
--- a/project/effect.cpp
+++ b/project/effect.cpp
@@ -351,7 +351,7 @@ void Effect::copy_field_keyframes(EffectPtr e) {
   for (int i=0;i<rows.size();i++) {
     EffectRow* row = rows.at(i);
     EffectRow* copy_row = e->rows.at(i);
-    copy_row->SetKeyframing(row->IsKeyframing());
+    copy_row->SetKeyframingInternal(row->IsKeyframing());
     for (int j=0;j<row->FieldCount();j++) {
       // Get field from this (the source) effect
       EffectField* field = row->Field(j);
@@ -390,8 +390,7 @@ int Effect::gizmo_count() {
 void Effect::refresh() {}
 
 void Effect::FieldChanged() {
-  panel_sequence_viewer->viewer_widget->frame_update();
-  panel_graph_editor->update_panel();
+  update_ui(false);
 }
 
 void Effect::delete_self() {
@@ -545,7 +544,7 @@ void Effect::load(QXmlStreamReader& stream) {
 
                 // read keyframes
                 if (stream.name() == "key" && stream.isStartElement()) {
-                  row->SetKeyframing(true);
+                  row->SetKeyframingInternal(true);
 
                   EffectKeyframe key;
                   for (int k=0;k<stream.attributes().size();k++) {
@@ -623,7 +622,7 @@ void Effect::load_from_string(const QByteArray &s) {
   // clear existing keyframe data
   for (int i=0;i<rows.size();i++) {
     EffectRow* row = rows.at(i);
-    row->SetKeyframing(false);
+    row->SetKeyframingInternal(false);
     for (int j=0;j<row->FieldCount();j++) {
       EffectField* field = row->Field(j);
       field->keyframes.clear();
diff --git a/project/effectfields/effectfield.cpp b/project/effectfields/effectfield.cpp
index a3db48cea..0fe9421a1 100644
--- a/project/effectfields/effectfield.cpp
+++ b/project/effectfields/effectfield.cpp
@@ -125,26 +125,58 @@ QVariant EffectField::GetValueAt(double timecode)
         double after_dbl = after_key.data.toDouble();
 
         if (before_key.type == EFFECT_KEYFRAME_HOLD) {
-          // hold
+
+          // Hold keyframes will always return the previous keyframe with no interpolation
           value = before_dbl;
+
         } else if (before_key.type == EFFECT_KEYFRAME_BEZIER || after_key.type == EFFECT_KEYFRAME_BEZIER) {
+
           // bezier interpolation
           if (before_key.type == EFFECT_KEYFRAME_BEZIER && after_key.type == EFFECT_KEYFRAME_BEZIER) {
+
             // cubic bezier
-            double t = cubic_t_from_x(timecode*GetParentRow()->GetParentEffect()->parent_clip->sequence->frame_rate, before_key.time, before_key.time+GetValidKeyframeHandlePosition(before_keyframe, true), after_key.time+GetValidKeyframeHandlePosition(after_keyframe, false), after_key.time);
-            value = cubic_from_t(before_dbl, before_dbl+before_key.post_handle_y, after_dbl+after_key.pre_handle_y, after_dbl, t);
+            double t = cubic_t_from_x(timecodeToFrame(timecode),
+                                      before_key.time,
+                                      before_key.time+GetValidKeyframeHandlePosition(before_keyframe, true),
+                                      after_key.time+GetValidKeyframeHandlePosition(after_keyframe, false),
+                                      after_key.time);
+
+            value = cubic_from_t(before_dbl,
+                                 before_dbl+before_key.post_handle_y,
+                                 after_dbl+after_key.pre_handle_y,
+                                 after_dbl,
+                                 t);
+
           } else if (after_key.type == EFFECT_KEYFRAME_LINEAR) { // quadratic bezier
+
             // last keyframe is the bezier one
-            double t = quad_t_from_x(timecode*GetParentRow()->GetParentEffect()->parent_clip->sequence->frame_rate, before_key.time, before_key.time+GetValidKeyframeHandlePosition(before_keyframe, true), after_key.time);
-            value = quad_from_t(before_dbl, before_dbl+before_key.post_handle_y, after_dbl, t);
+            double t = quad_t_from_x(timecodeToFrame(timecode),
+                                     before_key.time,
+                                     before_key.time+GetValidKeyframeHandlePosition(before_keyframe, true),
+                                     after_key.time);
+
+            value = quad_from_t(before_dbl,
+                                before_dbl+before_key.post_handle_y,
+                                after_dbl,
+                                t);
+
           } else {
             // this keyframe is the bezier one
-            double t = quad_t_from_x(timecode*GetParentRow()->GetParentEffect()->parent_clip->sequence->frame_rate, before_key.time, after_key.time+GetValidKeyframeHandlePosition(after_keyframe, false), after_key.time);
-            value = quad_from_t(before_dbl, after_dbl+after_key.pre_handle_y, after_dbl, t);
+            double t = quad_t_from_x(timecodeToFrame(timecode),
+                                     before_key.time,
+                                     after_key.time+GetValidKeyframeHandlePosition(after_keyframe, false),
+                                     after_key.time);
+
+            value = quad_from_t(before_dbl,
+                                after_dbl+after_key.pre_handle_y,
+                                after_dbl,
+                                t);
           }
         } else {
-          // linear
+
+          // Linear interpolation (default)
           value = double_lerp(before_dbl, after_dbl, progress);
+
         }
       }
       return value;
@@ -175,7 +207,7 @@ QVariant EffectField::GetValueAt(double timecode)
   return keyframes.first().data;
 }
 
-void EffectField::SetValueAt(double timecode, const QVariant &value)
+void EffectField::SetValueAt(double time, const QVariant &value)
 {
   if (keyframes.isEmpty()) {
     EffectKeyframe key;
@@ -185,7 +217,32 @@ void EffectField::SetValueAt(double timecode, const QVariant &value)
   }
 
   if (GetParentRow()->IsKeyframing()) {
-    // create keyframe here
+
+    // Create keyframe here
+
+    // Convert seconds timecode to frame
+    long frame_timecode = timecodeToFrame(time);
+
+    // Check array if a keyframe at this time already exists
+    int keyframe_index = -1;
+    for (int i=0;i<keyframes.size();i++) {
+      if (keyframes.at(i).time == frame_timecode) {
+        keyframe_index = i;
+        break;
+      }
+    }
+
+    // If keyframe doesn't exist, make it
+    if (keyframe_index == -1) {
+      EffectKeyframe key;
+      key.time = frame_timecode;
+      key.data = value;
+      keyframes.append(key);
+    } else {
+      EffectKeyframe& key = keyframes[keyframe_index];
+      key.data = value;
+    }
+
   } else {
     keyframes.first().data = value;
   }
@@ -199,6 +256,28 @@ double EffectField::Now()
   return playhead_to_clip_seconds(c, c->sequence->playhead);
 }
 
+void EffectField::PrepareDataForKeyframing(bool enabled, ComboAction *ca)
+{
+  if (enabled) {
+
+    // Convert "perpetual" keyframe to a keyframe at this time
+    ca->append(new SetLong(&keyframes.first().time, keyframes.first().time, timecodeToFrame(Now())));
+
+  } else {
+
+    // Convert keyframes to one "perpetual" keyframe
+
+    // Set first keyframe to whatever the data is now
+    ca->append(new SetQVariant(&keyframes.first().data, keyframes.first().data, GetValueAt(Now())));
+
+    // Delete all keyframes except the first
+    for (int i=1;i<keyframes.size();i++) {
+      ca->append(new KeyframeDelete(this, 1));
+    }
+
+  }
+}
+
 const EffectField::EffectFieldType &EffectField::type()
 {
   return type_;
diff --git a/project/effectfields/effectfield.h b/project/effectfields/effectfield.h
index b3fb60a29..08fce79bb 100644
--- a/project/effectfields/effectfield.h
+++ b/project/effectfields/effectfield.h
@@ -53,10 +53,12 @@ public:
   const QString& id();
 
   QVariant GetValueAt(double timecode);
-  void SetValueAt(double timecode, const QVariant& value);
+  void SetValueAt(double time, const QVariant& value);
 
   double Now();
 
+  void PrepareDataForKeyframing(bool enabled, ComboAction* ca);
+
   int GetColumnSpan();
   void SetColumnSpan(int i);
 
@@ -69,8 +71,15 @@ public:
 
   bool IsEnabled();
   void SetEnabled(bool e);
+
   QVector<EffectKeyframe> keyframes;
 
+signals:
+  void Changed();
+  void Clicked();
+
+  void EnabledChanged(bool);
+
 private:
   EffectFieldType type_;
   QString id_;
@@ -82,11 +91,7 @@ private:
 
   bool enabled_;
   int colspan_;
-signals:
-  void Changed();
-  void Clicked();
 
-  void EnabledChanged(bool);
 };
 
 #endif // EFFECTFIELD_H
diff --git a/project/effectrow.cpp b/project/effectrow.cpp
index 779f97051..a1337fb7b 100644
--- a/project/effectrow.cpp
+++ b/project/effectrow.cpp
@@ -58,7 +58,7 @@ bool EffectRow::IsKeyframing() {
   return keyframing_;
 }
 
-void EffectRow::SetKeyframing(bool b) {
+void EffectRow::SetKeyframingInternal(bool b) {
   if (GetParentEffect()->meta->type != EFFECT_TYPE_TRANSITION) {
     keyframing_ = b;
     emit KeyframingSetChanged(keyframing_);
@@ -72,28 +72,46 @@ bool EffectRow::IsSavable()
 
 void EffectRow::SetKeyframingEnabled(bool enabled) {
   if (enabled) {
+
     ComboAction* ca = new ComboAction();
+
+    // Enable keyframing setting on this row
     ca->append(new SetIsKeyframing(this, true));
-    set_keyframe_now(ca);
+
+    // Prepare each field's data to start keyframing
+    for (int i=0;i<FieldCount();i++) {
+      Field(i)->PrepareDataForKeyframing(true, ca);
+    }
+
     olive::UndoStack.push(ca);
+
   } else {
+
+    // Confirm with the user whether they really want to disable keyframing
     if (QMessageBox::question(panel_effect_controls,
                               tr("Disable Keyframes"),
-                              tr("Disabling keyframes will delete all current keyframes. Are you sure you want to do this?"),
+                              tr("Disabling keyframes will delete all current keyframes. "
+                                 "Are you sure you want to do this?"),
                               QMessageBox::Yes, QMessageBox::No) == QMessageBox::Yes) {
-      // clear
+
       ComboAction* ca = new ComboAction();
+
+      // Prepare each field's data to stop keyframing
       for (int i=0;i<FieldCount();i++) {
-        EffectField* f = Field(i);
-        for (int j=0;j<f->keyframes.size();j++) {
-          ca->append(new KeyframeDelete(f, 0));
-        }
+        Field(i)->PrepareDataForKeyframing(false, ca);
       }
+
+      // Disable keyframing setting on this row
       ca->append(new SetIsKeyframing(this, false));
+
       olive::UndoStack.push(ca);
       panel_effect_controls->update_keyframes();
+
     } else {
-      SetKeyframing(true);
+
+
+      SetKeyframingInternal(true);
+
     }
   }
 }
@@ -101,42 +119,73 @@ void EffectRow::SetKeyframingEnabled(bool enabled) {
 void EffectRow::GoToPreviousKeyframe() {
   long key = LONG_MIN;
   Clip* c = GetParentEffect()->parent_clip;
+  long sequence_playhead = c->sequence->playhead;
+
+  // Used to convert clip frame number to sequence frame number
+  long time_adjustment = c->timeline_in() - c->clip_in();
+
+  // Loop through all of this row's fields
   for (int i=0;i<FieldCount();i++) {
+
     EffectField* f = Field(i);
+
+    // Loop through all of this field's keyframes for a keyframe EARLIER than the playhead
     for (int j=0;j<f->keyframes.size();j++) {
-      long comp = f->keyframes.at(j).time - c->clip_in() + c->timeline_in();
-      if (comp < olive::ActiveSequence->playhead) {
+      long comp = f->keyframes.at(j).time + time_adjustment;
+
+      // Get the closest keyframe
+      if (comp < sequence_playhead) {
         key = qMax(comp, key);
       }
     }
   }
+
+  // If we found a keyframe less than the playhead, jump to it
   if (key != LONG_MIN) panel_sequence_viewer->seek(key);
 }
 
 void EffectRow::ToggleKeyframe() {
+  Clip* c = GetParentEffect()->parent_clip;
+  long sequence_playhead = c->sequence->playhead;
+
+  // Used to convert clip frame number to sequence frame number
+  long time_adjustment = c->timeline_in() - c->clip_in();
+
   QVector<EffectField*> key_fields;
   QVector<int> key_field_index;
-  Clip* c = GetParentEffect()->parent_clip;
+
+  // See if any keyframes on any fields are at the current time
   for (int j=0;j<FieldCount();j++) {
     EffectField* f = Field(j);
     for (int i=0;i<f->keyframes.size();i++) {
-      long comp = c->timeline_in() - c->clip_in() + f->keyframes.at(i).time;
-      if (comp == olive::ActiveSequence->playhead) {
+      long comp = f->keyframes.at(i).time + time_adjustment;
+
+      if (comp == sequence_playhead) {
+
+        // Cache the keyframes if they are at the current time
         key_fields.append(f);
         key_field_index.append(i);
+
       }
     }
   }
 
   ComboAction* ca = new ComboAction();
-  if (key_fields.size() == 0) {
-    // keyframe doesn't exist, set one
-    set_keyframe_now(ca);
+
+  if (key_fields.isEmpty()) {
+
+    // If we didn't find any current keyframes, create one for each field
+    SetKeyframeOnAllFields(ca);
+
   } else {
+
+    // If we DID find keyframes at this time, delete them
     for (int i=0;i<key_fields.size();i++) {
       ca->append(new KeyframeDelete(key_fields.at(i), key_field_index.at(i)));
     }
+
   }
+
   olive::UndoStack.push(ca);
   update_ui(false);
 }
@@ -160,65 +209,13 @@ void EffectRow::FocusRow() {
   panel_graph_editor->set_row(this);
 }
 
-void EffectRow::set_keyframe_now(ComboAction* ca) {
-  // TODO address this...
-  /*
-  long time = olive::ActiveSequence->playhead
-      - parent_effect->parent_clip->timeline_in()
-      + parent_effect->parent_clip->clip_in();
-
-  if (!just_made_unsafe_keyframe) {
-    EffectKeyframe key;
-    key.time = time;
-
-    unsafe_keys.resize(fieldCount());
-    unsafe_old_data.resize(fieldCount());
-    key_is_new.resize(fieldCount());
-
-    for (int i=0;i<fieldCount();i++) {
-      EffectField* f = field(i);
-
-      int exist_key = -1;
-      int closest_key = 0;
-      for (int j=0;j<f->keyframes.size();j++) {
-        if (f->keyframes.at(j).time == time) {
-          exist_key = j;
-        } else if (f->keyframes.at(j).time < time
-               && f->keyframes.at(closest_key).time < f->keyframes.at(j).time) {
-          closest_key = j;
-        }
-      }
-      if (exist_key == -1) {
-        key.type = (f->keyframes.size() == 0) ? EFFECT_KEYFRAME_LINEAR : f->keyframes.at(closest_key).type;
-        key.data = f->GetCurrentValue();
-        unsafe_keys[i] = f->keyframes.size();
-        f->keyframes.append(key);
-        key_is_new[i] = true;
-      } else {
-        unsafe_keys[i] = exist_key;
-        key_is_new[i] = false;
-      }
-      unsafe_old_data[i] = f->GetCurrentValue();
-    }
-    just_made_unsafe_keyframe = true;
-  }
-
-  for (int i=0;i<fieldCount();i++) {
-    field(i)->keyframes[unsafe_keys.at(i)].data = field(i)->GetCurrentValue();
-  }
-
-  if (ca != nullptr)	{
-    for (int i=0;i<fieldCount();i++) {
-      if (key_is_new.at(i)) ca->append(new KeyframeFieldSet(field(i), unsafe_keys.at(i)));
-      ca->append(new SetQVariant(&field(i)->keyframes[unsafe_keys.at(i)].data, unsafe_old_data.at(i), field(i)->GetCurrentValue()));
-    }
-    unsafe_keys.clear();
-    unsafe_old_data.clear();
-    just_made_unsafe_keyframe = false;
+void EffectRow::SetKeyframeOnAllFields(ComboAction* ca) {
+  for (int i=0;i<FieldCount();i++) {
+    EffectField* field = Field(i);
+    field->SetValueAt(field->Now(), field->GetValueAt(field->Now()));
   }
 
   panel_effect_controls->update_keyframes();
-  */
 }
 
 void EffectRow::delete_keyframe_at_time(ComboAction* ca, long time) {
diff --git a/project/effectrow.h b/project/effectrow.h
index a59486941..b3be99aed 100644
--- a/project/effectrow.h
+++ b/project/effectrow.h
@@ -45,7 +45,7 @@ public:
 
   EffectField* Field(int i);
   int FieldCount();
-  void set_keyframe_now(ComboAction *ca);
+  void SetKeyframeOnAllFields(ComboAction *ca);
   void delete_keyframe_at_time(ComboAction *ca, long time);
 
   Effect* GetParentEffect();
@@ -53,7 +53,7 @@ public:
   const QString& name();
 
   bool IsKeyframing();
-  void SetKeyframing(bool);
+  void SetKeyframingInternal(bool);
 
   bool IsSavable();
 public slots:
diff --git a/project/undo.cpp b/project/undo.cpp
index 34bf06794..1aa79eab3 100644
--- a/project/undo.cpp
+++ b/project/undo.cpp
@@ -1092,11 +1092,11 @@ SetIsKeyframing::SetIsKeyframing(EffectRow *irow, bool ib) {
 }
 
 void SetIsKeyframing::doUndo() {
-  row->SetKeyframing(!b);
+  row->SetKeyframingInternal(!b);
 }
 
 void SetIsKeyframing::doRedo() {
-  row->SetKeyframing(b);
+  row->SetKeyframingInternal(b);
 }
 
 RefreshClips::RefreshClips(Media *m) {
diff --git a/ui/keyframenavigator.cpp b/ui/keyframenavigator.cpp
index bf90d66e1..18625b26d 100644
--- a/ui/keyframenavigator.cpp
+++ b/ui/keyframenavigator.cpp
@@ -60,7 +60,8 @@ KeyframeNavigator::KeyframeNavigator(QWidget *parent, bool addLeftPad) : QWidget
   connect(right_key_nav, SIGNAL(clicked(bool)), this, SIGNAL(goto_next_key()));
   connect(right_key_nav, SIGNAL(clicked(bool)), this, SIGNAL(clicked()));
 
-  keyframe_enable = new QPushButton(olive::icon::Clock, "", this);
+  keyframe_enable = new QPushButton(this);
+  keyframe_enable->setIcon(olive::icon::Clock);
   keyframe_enable->setSizePolicy(QSizePolicy::Maximum, QSizePolicy::Fixed);
   keyframe_enable->setIconSize(keyframe_enable->iconSize()*0.75);
   keyframe_enable->setCheckable(true);
diff --git a/ui/keyframenavigator.h b/ui/keyframenavigator.h
index c82b7344d..b0141aeb1 100644
--- a/ui/keyframenavigator.h
+++ b/ui/keyframenavigator.h
@@ -28,26 +28,26 @@ class QPushButton;
 
 class KeyframeNavigator : public QWidget
 {
-	Q_OBJECT
+  Q_OBJECT
 public:
-	KeyframeNavigator(QWidget* parent = 0, bool addLeftPad = true);
-	~KeyframeNavigator();
-	void enable_keyframes(bool);
-	void enable_keyframe_toggle(bool);
+  KeyframeNavigator(QWidget* parent = nullptr, bool addLeftPad = true);
+  ~KeyframeNavigator();
+  void enable_keyframes(bool);
+  void enable_keyframe_toggle(bool);
 signals:
-	void goto_previous_key();
-	void toggle_key();
-	void goto_next_key();
-	void keyframe_enabled_changed(bool);
-	void clicked();
+  void goto_previous_key();
+  void toggle_key();
+  void goto_next_key();
+  void keyframe_enabled_changed(bool);
+  void clicked();
 private slots:
-	void keyframe_ui_enabled(bool);
+  void keyframe_ui_enabled(bool);
 private:
-	QHBoxLayout* key_controls;
-	QPushButton* left_key_nav;
-	QPushButton* key_addremove;
-	QPushButton* right_key_nav;
-	QPushButton* keyframe_enable;
+  QHBoxLayout* key_controls;
+  QPushButton* left_key_nav;
+  QPushButton* key_addremove;
+  QPushButton* right_key_nav;
+  QPushButton* keyframe_enable;
 };
 
 #endif // KEYFRAMENAVIGATOR_H