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.

Sign up for GitHub

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

Jump to bottom

RE: Fix UI Layout #66

Merged
merged 13 commits into from
Jul 29, 2018
Merged

RE: Fix UI Layout #66

merged 13 commits into from
Jul 29, 2018

Conversation

Adyel
Copy link
Member

@Adyel Adyel commented Jul 23, 2018

  • Fixed conflict
  • Fixed code formatting
  • Added UI Layout

@Adyel Adyel requested a review from mominul July 23, 2018 15:59
@Adyel
Copy link
Member Author

Adyel commented Jul 23, 2018

@mominul Please do a final check before pushing to the main branch.

@Adyel Adyel mentioned this pull request Jul 23, 2018
Closed
mominul
mominul previously requested changes Jul 24, 2018
View reviewed changes
src/frontend/SettingsDialog.ui Outdated
@@ -1,178 +1,133 @@
<?xml version="1.0" encoding="UTF-8"?>
<ui version="4.0">
Copy link
Member

Choose a reason for hiding this comment

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

settings_cutoff
The text is being cut off.

src/frontend/AboutDialog.ui Outdated
@@ -1,869 +1,2568 @@
<?xml version="1.0" encoding="UTF-8"?>
Copy link
Member

Choose a reason for hiding this comment

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

too_much_about
Too much spaces!

Copy link
Member Author

Choose a reason for hiding this comment

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

This is OLD School one, I'm not gonna change it's layout for now. It requires some rework.

src/frontend/AboutFile.ui
@@ -6,153 +6,152 @@
<rect>
Copy link
Member

Choose a reason for hiding this comment

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

about_layout_mistakes
That Ok button is too much wide :)

src/frontend/AutoCorrectDialog.ui Outdated
@@ -1,228 +1,167 @@
<?xml version="1.0" encoding="UTF-8"?>
Copy link
Member

Choose a reason for hiding this comment

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

autocorrect_mistakes
That two text boxes are unequal in size. Clear button is also quite inconsistent.

@Adyel
Copy link
Member Author

Adyel commented Jul 26, 2018

@mominul
Fixed the look

AboutFile.ui
image

SettingsDialog.ui
This looked fine on my End. After your report I added padding just in case. Please, recheck if it is fixed.
image

AutoCorrectDialog.ui
image

@Adyel Adyel dismissed mominul’s stale review July 26, 2018 06:40

Please, Review Again. Updated.

@Adyel
Copy link
Member Author

Adyel commented Jul 27, 2018

@mominul So, looks ok? Merge?

@mominul
Copy link
Member

mominul commented Jul 27, 2018

Sorry I haven't tested out the changes yet! 😫

mominul
mominul requested changes Jul 27, 2018
View reviewed changes
src/frontend/SettingsDialog.ui
@@ -13,132 +13,139 @@
<property name="windowTitle">
Copy link
Member

Choose a reason for hiding this comment

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

settings_flaw
The padding didn't work. There are now two lines which are being cut off.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, That's weird. This window supposes to have a fixed width. It is showing way smaller in your screenshot.

Copy link
Member Author

Choose a reason for hiding this comment

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

@mominul Can you come to discord.

@mominul
Copy link
Member

mominul commented Jul 29, 2018

Merging this now. Will have further discussions at #56 . Thanks for the work @Adyel !

@mominul mominul merged commit df94e5e into OpenBangla:master Jul 29, 2018
@Adyel Adyel added Solved The issue is already solved and removed Reviewing Enhancement labels Jul 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mominul mominul mominul approved these changes

Assignees
No one assigned
Labels
Enhancement Solved The issue is already solved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Adyel @mominul