Skip to content

Conversation

@Kelketek
Copy link
Member

@antoviaque Hopefully these should address your notes.

Note, instead of making an extra download button, I placed the link where you suggested. Adding a download button would have prompted a lot more work in terms of alignment of the pop-ups and notifications, and I felt that it would be better to just place it down below.

@Kelketek
Copy link
Member Author

@bradenmacdonald And you should probably verify these as well, come to think of it.

Copy link
Member

Choose a reason for hiding this comment

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

Did you intend to use this event parameter, or are you just adding it for consistency?

@bradenmacdonald
Copy link
Member

@Kelketek

  • I think maybe the "Share" button should be disabled when viewing another user's table.
  • When I view another user's table and press "Download", it downloads my table instead. The download link should probably also be disabled.
  • There are still too many "+" buttons visible:
    screen shot 2015-07-21 at 12 16 43 pm
  • I think it's slightly confusing to say "Enter usernames" when there is only one text field visible - users may try to enter multiple usernames into the field. Or maybe I'm being too pessimistic.
  • Personally, I preferred this before when it had a bit of an explanation and didn't just say "Enter usernames:"

@bradenmacdonald
Copy link
Member

Looks good! 👍 if you get tests passing. Failures look unrelated so maybe just run the builds again.

@Kelketek
Copy link
Member Author

@bradenmacdonald I've rerun the tests a few times. Each time the failure is somewhere else but not related to the current changes, usually appearing to be something with upstream libraries, like getting more than one block or a transaction error. I can't seem to restart it anymore. Since all the tests have passed individually, I'm going to go ahead and merge so @mtyaka can complete his end of things.

Kelketek added a commit that referenced this pull request Jul 22, 2015
Visual improvements to Table block.
@Kelketek Kelketek merged commit 1da02fe into master Jul 22, 2015
@Kelketek Kelketek deleted the download-table branch July 22, 2015 00:59
@bradenmacdonald
Copy link
Member

@Kelketek Can you please create a JIRA ticket then about the flaky tests?

@Kelketek
Copy link
Member Author

@bradenmacdonald A ticket has been added to the backlog.

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.

3 participants