Skip to content
This repository has been archived by the owner on May 24, 2022. It is now read-only.

Fixed window size #436

Merged
merged 30 commits into from
Feb 25, 2019
Merged

Fixed window size #436

merged 30 commits into from
Feb 25, 2019

Conversation

Tbaut
Copy link
Collaborator

@Tbaut Tbaut commented Feb 20, 2019

closes #275
closes #366

Todo:

  • Tweak Tx signature screen to avoid scroll bar
  • Tweak Signer scanner to avoid scroll bar
  • Remove comments
  • Test on other platforms

Issues on Windows:

  • window is 446px on windows (instead of 515px) - was because of the frame.
  • On Windows, Fether window is getting re-sized automatically (shrunk) when hidding/showing it! (handled in a separate PR)

@Tbaut Tbaut changed the title Fix window size Fixed window size Feb 21, 2019
@Tbaut
Copy link
Collaborator Author

Tbaut commented Feb 21, 2019

On Windows, fether window is getting re-sized automatically (shrunk) when hidding/showing it!

Maybe @ltfschoen can help me on that, I figured it also happens on master. To reproduce:

  • click several times on the taskbar icon, so that is shows and hide Fether. Notice how the windows shrinks.

@ltfschoen
Copy link
Contributor

ltfschoen commented Feb 21, 2019

On Windows, fether window is getting re-sized automatically (shrunk) when hidding/showing it!

Maybe @ltfschoen can help me on that, I figured it also happens on master. To reproduce:

  • click several times on the taskbar icon, so that is shows and hide Fether. Notice how the windows shrinks.

Yes, that's because at the moment we're showing a "frame" on Windows OS and the user click press ALT to toggle showing or hiding the menu. But when the menu appeared it was cropping the "Feedback" button, so we tried to reduce the height of the Fether window when the Fether menu was shown, and increase the height of the Fether window when the Fether menu was hidden again. We did that change to fether-electron/src/main/app/messages/index.js in this commit

EDIT: commit b4318da

@Tbaut
Copy link
Collaborator Author

Tbaut commented Feb 22, 2019

Tested on Windows and Linux, it should be good to go.
Can you give it a shot on Mac @ltfschoen and tell me if there's anything broken or with a scroll bar?
Here's how it's supposed to look like:

image

Note that the import screen (with JSON, recovery phrase and Parity Signer) has a scrollbar, it's not perfect but should be solved with #389.

<Header title={<h1>Terms of Use</h1>} />

<div className='window_content'>
<div className='box -padded-extra'>
Copy link
Contributor

Choose a reason for hiding this comment

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

consider removing -padded-extra from fether-react/src/assets/sass/shared/_box.scss if we're no longer using it

Copy link
Contributor

@ltfschoen ltfschoen left a comment

Choose a reason for hiding this comment

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

LGTM

just some nitpicks. i think we should merge PR #443 first, then this PR, and then lastly #440

Note: Tested on Windows 10, Linux Mint and macOS. I tested it on macOS as follows:

  • macOS
    • Tested with yarn; yarn electron
    • Tested the T&C page by running rm -rf ~/Library/Application Support/Electron before starting app
    • Tested Accounts List page by only having a single key (no scroll) and multiple keys (scroll) in ~/Library/Application Support/io.parity.ethereum/keys/kovan
    • Tested Account (token list) page, both with single token (no scroll) and multiple tokens (scroll)
    • Tested Send Ether page, when you click "Details" it makes the page scroll.
      Suggest that we make the Tx Details section a popup, which was an option when it was originally created, see feat - Fixes #293. Shows tx fee estimate and total tx amount. See issues in commit details #307 (comment)

@Tbaut
Copy link
Collaborator Author

Tbaut commented Feb 25, 2019

The PR #427 broke my changes as the overlay applies to the whole app. (it adds the modal, hence a <div class="alert-wrapper" everywhere).
I need to refactor it in another PR.

@Tbaut
Copy link
Collaborator Author

Tbaut commented Feb 25, 2019

was fixed easiely by simply removing this div class="alert-wrapper" Thanks @axelchalon :)

@Tbaut Tbaut merged commit e641594 into master Feb 25, 2019
@Tbaut Tbaut deleted the tbaut-fix-window-size branch February 25, 2019 13:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make the T&C's take the full screen Need a constant screen size especially when loading an account's tokens
2 participants