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

App icon change #19

Closed
wants to merge 21 commits into from
Closed

App icon change #19

wants to merge 21 commits into from

Conversation

jacsonah
Copy link
Contributor

No description provided.

mpobaschnig and others added 18 commits May 3, 2021 23:27
* ui: add AddPage for adding/importing vaults

* user_config_manager: fix user config dir naming

* user_config_manager: add user data directory

* user_config_manager: add ending slash to data dir

* user_config_manager: add home dir

* user_config_manager: add getter for vaults home

* Move from add/import dialog to page with carousel

Since GtkDialogs are pretty bad on mobile devices, move add/import to
page. This also enables us to use a spinner within the add button.
Additional add async backend init. This is a rough implementation
and needs some additional polishing, but should be working fine.

* window: startup with StartPage in empty state

* AddPage: Correctly init combo_box

* window: go back to page depending on entries

* ui: Add revealer to show info about backends

* ui: ignore password when importing

* ui: Don't fill edd/md when importing

* AddPage: add file chooser dialogs

* AddPage: correctly validate edd validation

* AddPage: implement import button functionality

* AddPage: handle empty edd/md paths

* Move from password dialog to password page

* Move from settings dialog to settings page

* cleanup: Remove old files

* window: Cleanup

* ui: Set ui insensitive while waiting for add

* user_config_manager: fix name

* ui: Remove refresh button

* AddPage: Focus vault_name_entry when moving to it

* pages: Properly disconnect signals
This PR moves the previous/next buttons when adding/importing vaults to the headerbar. It also adds a home button to move back to the start/vault view.
Change the go-home-symbolic icon back to go-previous-symbolic.
Also, remove the go-home functionality when adding/importing after
the first page, so ones has to click back multiple times.
This actually looks better and is coherent with other apps.
@jacsonah jacsonah mentioned this pull request Oct 10, 2021
Copy link
Owner

@mpobaschnig mpobaschnig left a comment

Choose a reason for hiding this comment

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

Just had a closer look at the images again and the symmetry/alignment is still a bit off, e.g. outer shape horizontally/vertically (now it's left side ~12.4px, right side ~11.6px, top side ~12.5px, bottom side ~11.5px). Generally try to align lines/shapes to pixel grid in integer steps (see Password Safe Icon for example), then all lines/shapes should have same integer step distances to their respective side).

Then I think this is good to go.

@jacsonah
Copy link
Contributor Author

Sorry, but what tool did you use to measure these values? I'm a little confused about which part I should align.

@mpobaschnig
Copy link
Owner

I use Inkscape for that. Either via measurement tool or using rectangles to measure on the page grid. When turning on page grid, you can easily see symmetry or alignment issues, see these pictures, where the black rectangles are grid aligned:

Screenshot from 2021-10-30 18-07-27

Screenshot from 2021-10-30 18-12-08

Screenshot from 2021-10-30 18-14-06

As comparison, Password Safe icon alignment:

Screenshot from 2021-10-30 18-02-01

Basically every element should be aligned with the grid here, e.g. circles should align with top/left/bottom/right side, lines should align, and so on.

Copy link
Owner

@mpobaschnig mpobaschnig left a comment

Choose a reason for hiding this comment

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

The inner, bright part is now shifted by 2px to the bottom, hiding the "step"/darker shade.

Still missing:

  • Inner circles are not grid aligned
  • Metal bars are not grid aligned

Copy link
Owner

@mpobaschnig mpobaschnig left a comment

Choose a reason for hiding this comment

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

The inner, bright part is still shifted by 2px to the bottom, hiding the "step"/darker shade.

Moving this bright inner part and the cross/circles in the middle by 2px up, then it's ready.

@Crono23
Copy link

Crono23 commented Nov 21, 2021

Hi, sorry if I sound impatient. I've been watching this merge request and it's almost a month since progress was made. Is it going to be finished or may I propose my own icon design?

@mpobaschnig
Copy link
Owner

There is not much left from the design side, so I don't think it's worth the time working on a new icon now.

@jacsonah If it's okay for you, I'll just fix the outstanding issues myself and merge it.

@jacsonah
Copy link
Contributor Author

There is not much left from the design side, so I don't think it's worth the time working on a new icon now.

@jacsonah If it's okay for you, I'll just fix the outstanding issues myself and merge it.

You can fix the outstanding issues, it's ok for me.

@mpobaschnig
Copy link
Owner

Cherry picked commits and pushed to master, since the diff between branches is too big. Thanks a lot!

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