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

Peng billy/669 preferences page #761

Merged
merged 28 commits into from
Jun 4, 2018
Merged

Conversation

okwme
Copy link
Contributor

@okwme okwme commented May 29, 2018

Closes #ISSUE #669

Description: we updated the profile page to a preferences page with a new layout to accomodate for the new content. I forked peng's vue-field component to include a toggle. the repo points to my fork until peng accepts my PR

screenshot 2018-05-29 16 36 20

❤️ Thank you!

@okwme okwme requested review from faboweb, NodeGuy and nylira as code owners May 29, 2018 14:36
@nylira
Copy link
Contributor

nylira commented May 30, 2018

Looks good! The design of the toggle does stick out a bit. Let's make some changes.

  • We don't need a label in the toggle. On or off should be obvious to the user with a background color for "on".
  • Let's make it a little less wide.
  • The scrubber for the toggle shouldn't be so dark.

I can do these changes tomorrow, or you can do it @okwme if you want. Will merge the PR into @nylira/vue-input tomorrow too.

@jbibla
Copy link
Collaborator

jbibla commented May 30, 2018

We don't need a label in the toggle. On or off should be obvious to the user with a background color for "on".

green and red can be used to help

i just noticed the buttons are quite skinny too - is this on purpose?

@okwme
Copy link
Contributor Author

okwme commented May 30, 2018

anyone know why these test are failing on circle and not when run locally?
The words in the toggle can be removed in the options. without them the toggle becomes smaller too. i've just replaced them and also added some margins, not sure if they help tho...

as for the scrubber color, i was just following the slider's lead by using the --txt as the color. What should I replace it with?

@jbibla
Copy link
Collaborator

jbibla commented May 30, 2018

yarn test:e2e fails locally for me

@okwme
Copy link
Contributor Author

okwme commented May 30, 2018

light:
screenshot 2018-05-30 18 02 18 screenshot 2018-05-30 18 02 25
dark:
screenshot 2018-05-30 18 02 41 screenshot 2018-05-30 18 02 34
action shot:
screenshot 2018-05-30 18 02 44 screenshot 2018-05-30 18 02 50

@jbibla
Copy link
Collaborator

jbibla commented May 30, 2018

love the actions shots

@nylira
Copy link
Contributor

nylira commented May 31, 2018

Edit: resolved with yarn install

@nylira
Copy link
Contributor

nylira commented May 31, 2018

@jolesbi BINARY_PATH=$GOPATH/bin/gaiacli yarn test runs for me locally.

@nylira nylira force-pushed the peng-billy/669-preferences-page branch from 6fde759 to 68e8ad0 Compare May 31, 2018 06:28
@nylira
Copy link
Contributor

nylira commented May 31, 2018

@jolesbi the buttons are wider than usual on purpose. I thought that setting the button width to equal the input looks better visually, following a grid.

@jbibla
Copy link
Collaborator

jbibla commented May 31, 2018

BINARY_PATH=$GOPATH/bin/gaiacli passes for me locally too... is it possible that billy's version of gaia is the issue?

sudo wget https://okw.me/gaiacli-0.17.0-ubuntu -O ~/repo/gaia

@faboweb
Copy link
Collaborator

faboweb commented May 31, 2018

Found a bug:
The network selector always starts with 'Live Network' even if started in mocked mode.

@okwme
Copy link
Contributor Author

okwme commented Jun 1, 2018

@jolesbi @faboweb would you maybe check these too to see if @jolesbi right and it's the binary to blame for the circle tests?

sha256sum gaiacli-0.17.0-ubuntu
bcb88084a55875d1707231f9fc4ef19fdc4cc3defcb130acb51fa3415f407cf2  gaiacli-0.17.0-ubuntu
sha256sum gaiad-0.17.0-ubuntu
42818bcd0c3e02e548c92871cf8677e22a3216edacc5ac162b800744f3c31c7d  gaiad-0.17.0-ubuntu

@faboweb
Copy link
Collaborator

faboweb commented Jun 1, 2018

I am pretty sure it is not the binary. Logging in already passed before. The issue appears only after switching to the mocked connector.

@okwme
Copy link
Contributor Author

okwme commented Jun 1, 2018

got the bug @faboweb nice catch 🐞

@faboweb
Copy link
Collaborator

faboweb commented Jun 1, 2018

So the problem is, it doesn't really logout.

@faboweb
Copy link
Collaborator

faboweb commented Jun 1, 2018

The network switch is fixed. Now a test fails because it doesn't correctly check a checkbox.

@nylira
Copy link
Contributor

nylira commented Jun 4, 2018

TAP version 13
# preferences
using cli binary /Users/pz/go/bin/gaiacli
using node binary /Users/pz/go/bin/gaiad
ui home: /var/folders/v3/yzp94ctx4dzdzcnvt1q4_7qm0000gn/T/8suzom1895x
node home: /var/folders/v3/yzp94ctx4dzdzcnvt1q4_7qm0000gn/T/blau6kh90fp
/Users/pz/go/bin/gaiad start --home /var/folders/v3/yzp94ctx4dzdzcnvt1q4_7qm0000gn/T/blau6kh90fp
Started local node.
unhandledRejection Error: ChromeDriver did not start within 10000ms
    at /Users/pz/Projects/voyager/node_modules/spectron/lib/chrome-driver.js:63:25
    at Request._callback (/Users/pz/Projects/voyager/node_modules/spectron/lib/chrome-driver.js:120:23)
    at self.callback (/Users/pz/Projects/voyager/node_modules/request/request.js:186:22)
    at Request.emit (events.js:160:13)
    at Request.onRequestError (/Users/pz/Projects/voyager/node_modules/request/request.js:878:8)
    at ClientRequest.emit (events.js:160:13)
    at Socket.socketErrorListener (_http_client.js:389:9)
    at Socket.emit (events.js:160:13)
    at emitErrorNT (internal/streams/destroy.js:64:8)
    at process._tickCallback (internal/process/next_tick.js:152:19)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
pz@mbp ~/P/voyager> yarn
yarn install v1.7.0
[1/5] 🔍  Validating package.json...
[2/5] 🔍  Resolving packages...
[3/5] 🚚  Fetching packages...
[4/5] 🔗  Linking dependencies...
[5/5] 📃  Building fresh packages...
✨  Done in 4.82s.

Is this the yarn test:e2e error everyone is getting?

@faboweb
Copy link
Collaborator

faboweb commented Jun 4, 2018

Nope ;) Try reinstalling the dependencies. Billy also had problems with a newer Chrome version before. I am currently investigating the failing tests.

@codecov
Copy link

codecov bot commented Jun 4, 2018

Codecov Report

Merging #761 into develop will decrease coverage by 0.2%.
The diff coverage is 80.95%.

@@             Coverage Diff             @@
##           develop     #761      +/-   ##
===========================================
- Coverage    87.75%   87.55%   -0.21%     
===========================================
  Files           94       94              
  Lines         1601     1607       +6     
  Branches        88       91       +3     
===========================================
+ Hits          1405     1407       +2     
- Misses         182      184       +2     
- Partials        14       16       +2
Impacted Files Coverage Δ
app/src/renderer/routes.js 100% <ø> (ø) ⬆️
app/src/renderer/components/common/NiUserPane.vue 100% <ø> (ø) ⬆️
app/src/renderer/components/common/NiListItem.vue 100% <ø> (ø) ⬆️
app/src/renderer/components/monitor/PageBlock.vue 100% <ø> (ø) ⬆️
...src/renderer/components/common/PagePreferences.vue 80.95% <80.95%> (ø)

@faboweb faboweb merged commit 518e5cb into develop Jun 4, 2018
@faboweb faboweb deleted the peng-billy/669-preferences-page branch June 4, 2018 10:12
faboweb pushed a commit that referenced this pull request Jun 2, 2020
Bumps [prettier](https://github.com/prettier/prettier) from 1.19.1 to 2.0.5.
- [Release notes](https://github.com/prettier/prettier/releases)
- [Changelog](https://github.com/prettier/prettier/blob/master/CHANGELOG.md)
- [Commits](prettier/prettier@1.19.1...2.0.5)

Signed-off-by: dependabot-preview[bot] <[email protected]>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
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.

4 participants