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

(Feature) Apply new design to step 4: "Publish" #1204

Conversation

gabitoesmiapodo
Copy link

@gabitoesmiapodo gabitoesmiapodo commented Oct 29, 2018

Closes #1088

Description:

Here's the implementation of Step 4's styles.

There are some changes to the original designs (fields' order, added TxStatus elements) in the hopes of making everything clearer and easier to the end user.

Screenshots:

screen shot 2018-10-29 at 15 18 37
screen shot 2018-10-29 at 16 16 56
screen shot 2018-10-29 at 15 25 41
screen shot 2018-10-29 at 16 26 16
screen shot 2018-10-29 at 15 25 51
screen shot 2018-10-29 at 16 26 27

gabitoesmiapodo and others added 30 commits August 30, 2018 14:03
(Feature) Add assets to support new design
(feat) Loading component
(feat) Home page modal (in progress)
…sign-landing-page

* integration/#1082-new-design:
  new source of e2e-test: branch 2.0-newDesign
* 2.0:
  Remove ABI source code
  Remove bin from summary
  Add Proxy Contract contents
* 2.0:
  Set ABI-Encoded Params to Proxy constructor params
  Add ABI-Encoded params
* 2.0:
  Add proxyName, compiler version and optimization info
* 2.0: (171 commits)
  Remove conditional
  Add source code of smart proxy contract to zip archive for download
  Change how validate params and network id
  Check crowdsale env vars by crowdsale type
  Added new contracts keys to file
  Remove unused function
  Refactoring function
  Update snapshot
  Reload storage on home
  Change label on content file
  Add function import
  Sort crowdsales list
  Add clear storage on manage section
  Add alert info if contracts already deployed
  Update snapshot
  Change how gas type is set
  Validate network change
  Disable supply input on dutch strategy
  Supply disable
  Fix getAddr
  ...

# Conflicts:
#	package.json
#	src/components/Home/index.js
#	src/components/stepFour/index.js
#	src/components/stepOne/index.js
#	src/components/stepThree/index.js
…-new-design-step-1

* feature/#1084-new-design-landing-page: (179 commits)
  (fix) Firefox button borders.
  (feedback) Various fixes and changes from #1122
  Remove conditional
  Add proxyName, compiler version and optimization info
  Set ABI-Encoded Params to Proxy constructor params
  Add source code of smart proxy contract to zip archive for download
  Change how validate params and network id
  Check crowdsale env vars by crowdsale type
  Add ABI-Encoded params
  Added new contracts keys to file
  Remove unused function
  Refactoring function
  Update snapshot
  Reload storage on home
  Remove ABI source code
  Change label on content file
  Remove bin from summary
  Add function import
  Sort crowdsales list
  Add clear storage on manage section
  ...

# Conflicts:
#	src/assets/stylesheets/styles.css
- added method to handle navigation to facilitate testing
…-new-design-step-1

* feature/#1084-new-design-landing-page:
  Add tests for Home component - added method to handle navigation to facilitate testing
  Add jest-localstorage-mock as dev-dependency
  Update CrowdsaleList component tests
  Update Loader component tests
  Remove unused components and its tests
  (fix) Small layout / modal CSS fix.

# Conflicts:
#	package.json
#	src/assets/stylesheets/styles.css
…ding-page

(Feature) New design for home / landing page
* 2.0:
  Remove unused scripts
  Fix typo
  Add metadata to file
Copy link
Collaborator

@vbaranov vbaranov left a comment

Choose a reason for hiding this comment

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

@gabitoesmiapodo

  1. screen shot 2018-10-30 at 19 36 04
    column with tx status is not fixed. It is moving depending on the length of the current status message. Could we fix it?
  2. There is no response on copy action. Could we add a tooltip?
  3. screen shot 2018-10-30 at 19 43 45
  4. the same:
    screen shot 2018-10-30 at 19 45 39

@dennis00010011b
Copy link

@gabitoesmiapodo
Multitiers crowdsale: only Tier1 column is displayed in modal.
screen shot 2018-10-30 at 10 11 24 am

@dennis00010011b
Copy link

@gabitoesmiapodo
Extra title, shouldn't be here
screen shot 2018-10-30 at 10 41 15 am

@dennis00010011b
Copy link

@gabitoesmiapodo
Typo in Step3
screen shot 2018-10-30 at 11 00 14 am

@dennis00010011b
Copy link

dennis00010011b commented Oct 30, 2018

@gabitoesmiapodo
Dutch: tier 's info is placed in chaotic order. Would be better:
Start time - End Time
Global min cap - max cap
Min rate - max rate
Enable whitelisting

screen shot 2018-10-30 at 11 22 48 am

…new-design-to-step-4-publish

* integration/#1082-new-design:
  Added tests to webStore, and fake provider
  Remove try catch and fix linter
  WIP tests
  Refactoring
  Update link
  Refactoring
  Refactoring
  Remove spec
  Move switch , remove load
  Update DApp loading code considering upcoming breaking change in metaMask
  Set default custom price
  Check if address is changed
@gabitoesmiapodo
Copy link
Author

@vbaranov

1- column with tx status is not fixed. It is moving depending on the length of the current status message. Could we fix it? -> The "Tx Name" column's width is now fixed, so no columns should move now. I don't know if there's a better fix for this, other than changing how this information is shown.

2- There is no response on copy action. Could we add a tooltip? -> Yeah, now a message is shown when the user clicks on this.

3- Text in one line -> Sure. I think there was a mistake and some text got repeated.

4- Text in one line for wallet addres -> Sure, but for "Whitelist with cap" only, because there's a column for "Burn excess" next to in "Dutch Auction".

@dennis00010011b

  • Multitiers crowdsale: only Tier1 column is displayed in modal. -> I think only first tier and tiers with a whitelist are displayed (maybe someone can confirm this?)

@dennis00010011b
Copy link

@gabitoesmiapodo

  1. Yes, you are right
  • Multitiers crowdsale: only Tier1 column is displayed in modal. -> I think only first tier and tiers with a whitelist are displayed (maybe someone can confirm this?)

2.There is no modal in Step 4 after this commit 6ba5737

screen shot 2018-10-31 at 7 11 21 pm

@gabitoesmiapodo
Copy link
Author

@dennis00010011b

1- Extra title, shouldn't be here

Removed.

2- Typo in Step3

Fixed.

3- Dutch: tier 's info is placed in chaotic order. Would be better:

Start time      ---- End Time
Global min cap  ---- max cap
Min rate        ---- max rate
Enable whitelisting

Fixed.

4- There is no modal in Step 4 after this commit 6ba5737

Yeah, now it's back.

@dennis00010011b
Copy link

dennis00010011b commented Nov 1, 2018

@gabitoesmiapodo
LGTM, except two things:

  1. I still cannot compile build on MacOS unless to rename folder from src/components/crowdsale to src/components/Crowdsale`
    (Feature) Apply new design to step 5: "Crowdsale page" #1206 (comment)

  2. Should be mentioned about all tiers in modal? Example, crowdsale has 3 tiers: 1&3 without whitelist, 2nd whiteist, as a result the Tier3 isn't displayed. Would be better to group no-whitelisted tiers together?

screen shot 2018-11-01 at 3 49 21 pm

@gabitoesmiapodo
Copy link
Author

@dennis00010011b

I still cannot compile build on MacOS unless to rename folder from src/components/crowdsale to src/components/Crowdsale

I used another way to change the files / folders' names on MacOS and have them recognized by Git, please have a look now.

screen_shot_2018-11-02_at_08_35_35

Should be mentioned about all tiers in modal? Example, crowdsale has 3 tiers: 1&3 without whitelist, 2nd whiteist, as a result the Tier3 isn't displayed. Would be better to group no-whitelisted tiers together?

We should ask @vbaranov or maybe @fernandomg about this. I'm not particularly fond about the way this information is displayed now, for several reasons: It's not clear that only tiers with a whitelist are included, tiers' names aren't displayed, I can't cancel or leave this section if something goes wrong (or if I just want to), and generally speaking It's not always clear what's going on (to me at least).

Maybe we should just add a short text clarifying that only tiers' with a whitelist are included here? That would be a small improvement, but I'm not sure if that's really the problem.

I created an issue with a proposal of what I think would be a better way to display all this information to the user: #1208

@ghost ghost assigned fernandomg Nov 2, 2018
@fernandomg
Copy link
Contributor

@gabitoesmiapodo I just added a fix to eliminate the duplicated in-progress button

image

The rest looks fantastic.

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.

6 participants