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

Added a star icon for unsaved file name, added 'Saved: xx time ago' #177

Merged
merged 8 commits into from Nov 9, 2016

Conversation

Projects
None yet
3 participants
@yining1023
Member

yining1023 commented Nov 2, 2016

fixed #157 #158
screen shot 2016-11-02 at 4 26 33 pm

@catarak

This comment has been minimized.

Show comment
Hide comment
@catarak

catarak Nov 3, 2016

Member

I noticed that if you leave the editor running and don't touch it, the last saved text won't get updated as time passes. I think this is because it doesn't know it has to rerender. I think the way to fix this is:

  1. Make the time display into its own component (so that the entire Editor component doesn't have to rerender).
  2. In this component, call a function periodically that will force it to rerender, or update a prop periodically so that it will rerender.
    I know that's fairly high level, so I'm happy to go into detail with you about how to implement this.
Member

catarak commented Nov 3, 2016

I noticed that if you leave the editor running and don't touch it, the last saved text won't get updated as time passes. I think this is because it doesn't know it has to rerender. I think the way to fix this is:

  1. Make the time display into its own component (so that the entire Editor component doesn't have to rerender).
  2. In this component, call a function periodically that will force it to rerender, or update a prop periodically so that it will rerender.
    I know that's fairly high level, so I'm happy to go into detail with you about how to implement this.
@catarak

This comment has been minimized.

Show comment
Hide comment
@catarak

catarak Nov 3, 2016

Member

I also noticed that this causes a little styling weirdness:
screen shot 2016-11-02 at 10 06 43 pm
If you look closely, you can see that the Sidebar component has a border-top, which looks kind of funny on its own. I would either (1) add a right border so it looks okay for this merge, or (2) implement this how it looks in the design. If you want to do (1), then the new design can be implemented in another PR.

Member

catarak commented Nov 3, 2016

I also noticed that this causes a little styling weirdness:
screen shot 2016-11-02 at 10 06 43 pm
If you look closely, you can see that the Sidebar component has a border-top, which looks kind of funny on its own. I would either (1) add a right border so it looks okay for this merge, or (2) implement this how it looks in the design. If you want to do (1), then the new design can be implemented in another PR.

@yining1023

This comment has been minimized.

Show comment
Hide comment
@yining1023

yining1023 Nov 3, 2016

Member

Got it. Thanks! I will do (1) (add a right border), and I'm excited about making a new component. 😄

Member

yining1023 commented Nov 3, 2016

Got it. Thanks! I will do (1) (add a right border), and I'm excited about making a new component. 😄

@catarak

This comment has been minimized.

Show comment
Hide comment
@catarak

catarak Nov 4, 2016

Member

This PR has also alerted me to the fact that there's a bug with unsavedChanges:
(1) edit one file, unsavedChanges is true
(2) switch to a new file, unsavedChanges becomes false, despite the fact that there are unsaved changes
But this is a separate issue that you don't have to fix in this PR 😄

Member

catarak commented Nov 4, 2016

This PR has also alerted me to the fact that there's a bug with unsavedChanges:
(1) edit one file, unsavedChanges is true
(2) switch to a new file, unsavedChanges becomes false, despite the fact that there are unsaved changes
But this is a separate issue that you don't have to fix in this PR 😄

@catarak catarak merged commit e86e9a0 into processing:master Nov 9, 2016

@yining1023 yining1023 deleted the yining1023:lastsaved branch Nov 9, 2016

@kaganjd

This comment has been minimized.

Show comment
Hide comment
@kaganjd

kaganjd Nov 16, 2016

Collaborator

@yining1023 and @catarak, I'm still not seeing the star next to unsaved projects even if I'm logged in. Is this what you see?
screen shot 2016-11-16 at 6 35 06 pm

Collaborator

kaganjd commented Nov 16, 2016

@yining1023 and @catarak, I'm still not seeing the star next to unsaved projects even if I'm logged in. Is this what you see?
screen shot 2016-11-16 at 6 35 06 pm

@yining1023

This comment has been minimized.

Show comment
Hide comment
@yining1023

yining1023 Nov 17, 2016

Member

I think if you type something in the editor, the star will appear.

Member

yining1023 commented Nov 17, 2016

I think if you type something in the editor, the star will appear.

@kaganjd

This comment has been minimized.

Show comment
Hide comment
@kaganjd

kaganjd Nov 17, 2016

Collaborator

Ohhh! Got it, thanks.

On Wed, Nov 16, 2016 at 8:55 PM, Yining Shi notifications@github.com
wrote:

I think if you type something in the editor, the star will appear.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#177 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AIx0Y0r9cXM1Z4dpBRQx3GwS0J-X-p7qks5q-7QNgaJpZM4Knt0D
.

Collaborator

kaganjd commented Nov 17, 2016

Ohhh! Got it, thanks.

On Wed, Nov 16, 2016 at 8:55 PM, Yining Shi notifications@github.com
wrote:

I think if you type something in the editor, the star will appear.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#177 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AIx0Y0r9cXM1Z4dpBRQx3GwS0J-X-p7qks5q-7QNgaJpZM4Knt0D
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment