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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

UI fixups #892

Merged
merged 3 commits into from Jun 27, 2020
Merged

UI fixups #892

merged 3 commits into from Jun 27, 2020

Conversation

msfjarvis
Copy link
Member

@msfjarvis msfjarvis commented Jun 27, 2020 •
edited

馃摙 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates

馃摐 Description

Fixes folder label not wrapping off at the correct bounds and the error drawable for wrong password overlapping the
view password icon.

馃挕 Motivation and Context

The first bug was reported to me by @xdevs23 and the second one was pointed out by @FabianHenneke

馃挌 How did you test it?

Visual verification

馃摑 Checklist

  • I formatted the code with the IDE's reformat action (Ctrl + Shift + L/Cmd + Shift + L)
  • I reviewed submitted code
  • I added a CHANGELOG entry if applicable

馃摳 Screenshots / GIFs

Before

screenshot-20200628-010816
screenshot-20200628-010028

After

screenshot-20200628-010911
screenshot-20200628-010923

@msfjarvis
GitOperation: Ensure we call the correct setError method
317eb00 
Without this we'd have overlapping password toggle and error drawable icons.
This issue was triggered because of Kotlin converting `setError` to its property syntax.
That caused things to be delegated to platform `TextView`'s `setError(String)` method as
opposed to `TextInputEditText`'s `setError(String, Drawable)`.

Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@msfjarvis
Fix overlapping labels for very long site names
ec3535b 
This would previously get laid under the child count and folder indicator icon.
Now it will eagerly split into separate lines as soon as it reaches the child
count view

Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@msfjarvis
update changelog
e7a3a27 
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@msfjarvis msfjarvis added this to the 1.10.0 milestone Jun 27, 2020
@Skrilltrax
Copy link
Member

Looks good but that error prompt isn't a part of Material 2.0. Shouldn't we use the material components?

@msfjarvis
Copy link
Member Author

Looks good but that error prompt isn't a part of Material 2.0. Shouldn't we use the material components?

What's the MDC equivalent?

@Skrilltrax
Copy link
Member

Skrilltrax commented Jun 27, 2020 •
edited

Looks good but that error prompt isn't a part of Material 2.0. Shouldn't we use the material components?

What's the MDC equivalent?

https://material.io/components/images/content/bd72a300ee8c3236b48ee86d5c7824c7.png

https://material.io/develop/android/components/text-fields/

@msfjarvis
Copy link
Member Author

Looks good but that error prompt isn't a part of Material 2.0. Shouldn't we use the material components?

What's the MDC equivalent?

https://material.io/components/images/content/bd72a300ee8c3236b48ee86d5c7824c7.png

https://material.io/develop/android/components/text-fields/

Tried setting app:errorEnabled and reverting my commit and it looks like the old broken state, gonna stick with what I have here for the time being.

Screenshot

screenshot-20200628-013038

@Skrilltrax
Copy link
Member

Skrilltrax commented Jun 27, 2020 •
edited

Tried setting app:errorEnabled and reverting my commit and it looks like the old broken state, gonna stick with what I have here for the time being.

Screenshot

Yeah, I guess you'll manually have to disable and enable view password toggle in case of an error. I don't have many issues with the new tooltip.

@msfjarvis msfjarvis merged commit 9fc5d33 into develop Jun 27, 2020
@msfjarvis msfjarvis deleted the ui-fixups branch June 27, 2020 20:15
msfjarvis added a commit that referenced this pull request Jun 27, 2020
@msfjarvis
Merge branch 'develop' into enhancement/refresh
596e037 
* develop:
  UI fixups (#892)
msfjarvis added a commit that referenced this pull request Jun 27, 2020
@msfjarvis
Merge branch 'develop' into feature/configure-pull-behavior
51ab89b 
* develop: (24 commits)
  UI fixups (#892)
  Update Public Suffix List data (#888)
  Use remembered credential even if it is empty (#880)
  Reset SSH passphrase after SSH key import (#885)
  Prevent cached passwords from being wiped (#884)
  build: uprev all dependencies (#882)
  github: update actions for updated branching logic
  Use a custom sshj config (#878)
  Remove API 30 from pull request test matrix (#879)
  Add Google Play/F-Droid badges to the README (#877)
  Retire Android Arsenal badge (#876)
  Add paragraphs to F-Droid summary (#874)
  Replace YAML with HTML in F-Droid fastlane summary (#873)
  Add relnotes for #871 (#872)
  Add org.gnu.icecat as a trusted multi-origin browser (#871)
  README: re-add F-Droid (#870)
  build: prepare next development version
  build: bump version to 1.9.0
  Prepare release 1.9.0
  PasswordCreationActivity: properly guard rename code
  ...

Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@msfjarvis
Copy link
Member Author

msfjarvis commented Jun 27, 2020 •
edited

image

Would this be better? I'm not strongly attached to either option. Forget it, this breaks things.

@Skrilltrax
Copy link
Member

Cool, the earlier one was better anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Skrilltrax Skrilltrax Skrilltrax approved these changes

@fmeum fmeum Awaiting requested review from fmeum

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.10.0
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@msfjarvis @Skrilltrax