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

Restructure the layout of SourceWidget #330

Merged
merged 1 commit into from
Apr 28, 2019

Conversation

sssoleileraaa
Copy link
Contributor

@sssoleileraaa sssoleileraaa commented Apr 25, 2019

Description

Resolves #280

This restructures SourceWidget to show timestamp, star, delete, and attachment icons in preparation for:

We have a separate issue for styling and polishing, which will fix off-by-one pixels issues, etc.

Summary of Changes

  • There is now a timestamp row. The change here is a proposal for the team, leaning on @ninavizz and @eloquence for feedback. I noticed when testing the changes that our "x days ago" humanized timestamp is cut off by the message preview. I suggest that, for now, we lower the timestamp so that we won't have this problem, and create a new Issue if we want a different solution. I could also place the timestamp somewhere else in this PR if we find another location that makes sense. See images:

Trying to match Zeplin:
sourcelist_higlight

My proposal until further discussion about how to handle the timestamp getting cut off:
sourcelist-proposal

  • Note: the images above are showing preview sample text that was not included in this PR.
  • There is now a gutter section that shows the star toggle button.
  • There is now a metadata section that shows the attachment icon which will be replaced by the delete icon when the source is selected in a follow-up PR for Add "Delete" button to Selected Source item in Source List #300.
  • Instead of placing the delete icon in a random location, I decided to remove it until Add "Delete" button to Selected Source item in Source List #300 is resolved. I can add it back if we want. I just don't know where to add it since it's spec'd out as an overlay.
  • There is now a fixed preview section that is empty since I need to implement messages previews in a follow-up PR for Preview most recent message or reply in source view #135. I noticed in Zeplin that the source list items are all the same height, which I need to talk to @ninavizz about to make sure my assumption that a blank message will not be resized to be smaller than items with messages.

Test

  1. Create messages with attachments and see that icons show up in the metadata section.
  2. Make sure you can see the star in the gutter section.
  3. Make sure each source item is the same height.
  4. Resize window a bunch to make sure things don't get cut off and look right.

Known issues

@sssoleileraaa
Copy link
Contributor Author

There is now a timestamp row.

To address this divergence from the Zeplin design, I created this issue: #332

need to talk to @ninavizz about to make sure my assumption that a blank message will not be resized to be smaller than items with messages

Talked to Nina and verified that we will keep the sourcelist items a fixed height and leave a blank space if there is no message (e.g. the source just attaches a document).

@sssoleileraaa sssoleileraaa force-pushed the create_class_for_starring branch from 0cc03b4 to c499c71 Compare April 25, 2019 22:31
@sssoleileraaa sssoleileraaa force-pushed the issue-280 branch 4 times, most recently from bd80fee to 94d27c3 Compare April 26, 2019 01:20
@heartsucker
Copy link
Contributor

Resize window a bunch to make sure things don't get cut off and look right.

There is still #331, but this is not a regression.

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.

2 participants