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

feat: draw clipart option in drawer #388

Merged
merged 1 commit into from
Jun 20, 2019

Conversation

adityastic
Copy link
Collaborator

@adityastic adityastic commented Jun 18, 2019

Fixes #385

Screenshots:
On Drawer:

After Opening:

Drawn and Saved:

ClipArts get Updated:

@adityastic adityastic force-pushed the patch-385 branch 3 times, most recently from ae626fb to ed6f06e Compare June 20, 2019 04:16
@adityastic adityastic changed the title WIP feat: draw badge option in drawer feat: draw clipart option in drawer Jun 20, 2019
@auto-label auto-label bot added the feature label Jun 20, 2019
@adityastic adityastic force-pushed the patch-385 branch 3 times, most recently from 28096f6 to b911f0d Compare June 20, 2019 04:29
@adityastic
Copy link
Collaborator Author

@mariobehling
Copy link
Member

Awesome! Please resolve conflicts.

@mariobehling
Copy link
Member

@adityastic Please add a "Draw Clipart Icon" at the end of the clipart list. This button should direct to the "Draw Clipart screen.

@adityastic
Copy link
Collaborator Author

adityastic commented Jun 20, 2019

@mariobehling On it

Copy link
Member

@liveHarshit liveHarshit left a comment

Choose a reason for hiding this comment

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

Reset lgtm 👍

if (drawableInfo.image is VectorDrawable)
ImageUtils.vectorToBitmap(drawableInfo.image)
else
(drawableInfo.image as BitmapDrawable).bitmap, 70)), 0, strToAppend.length, 33)
Copy link
Member

Choose a reason for hiding this comment

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

use smart cast instead: drawableInfo.image is BitmapDrawable

Copy link
Member

Choose a reason for hiding this comment

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

There are 3 more unsafe casts like that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There are 3 more unsafe casts like that.

on it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done, please review


<ImageView
android:layout_width="48dp"
android:layout_height="48dp"
Copy link
Member

Choose a reason for hiding this comment

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

Dimensions should define inside dimentsion reqources file.

Copy link
Member

Choose a reason for hiding this comment

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

There are more than one.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@liveHarshit yes updated all with the same

DrawFragment()
}

private val viewModel by viewModel<DrawViewModel>()
Copy link
Member

Choose a reason for hiding this comment

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

Too generic, it can be drawViewModel

@adityastic adityastic force-pushed the patch-385 branch 2 times, most recently from 95270e9 to 2c0c3aa Compare June 20, 2019 08:06
@adityastic
Copy link
Collaborator Author

@liveHarshit updated

@adityastic
Copy link
Collaborator Author

@mariobehling here is the update. Screenshot of the update:
385add

@adityastic
Copy link
Collaborator Author

adityastic commented Jun 20, 2019

@liveHarshit updated with the rest, please review

android:layout_alignParentTop="true"
android:layout_centerInParent="true"
android:layout_marginStart="50dp"
android:layout_marginEnd="50dp"
Copy link
Member

Choose a reason for hiding this comment

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

in dimens.xml

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated, @liveHarshit review

android:layout_height="wrap_content"
android:layout_alignParentBottom="true"
android:orientation="horizontal"
android:padding="10dp">
Copy link
Member

Choose a reason for hiding this comment

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

dimens.xml

<LinearLayout
android:layout_width="wrap_content"
android:layout_height="match_parent"
android:layout_margin="10dp"
Copy link
Member

Choose a reason for hiding this comment

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

dimens.xml

<LinearLayout
android:layout_width="wrap_content"
android:layout_height="match_parent"
android:layout_margin="10dp"
Copy link
Member

Choose a reason for hiding this comment

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

same

<LinearLayout
android:layout_width="wrap_content"
android:layout_height="match_parent"
android:layout_margin="10dp"
Copy link
Member

Choose a reason for hiding this comment

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

same

@adityastic
Copy link
Collaborator Author

@mariobehling @iamareebjamal peer review done

@adityastic
Copy link
Collaborator Author

@mariobehling ready to merge

pull bot pushed a commit to sahilsaha7773/badge-magic-android that referenced this pull request Jun 20, 2019
@mariobehling mariobehling merged commit 2ef6cd3 into fossasia:development Jun 20, 2019
@adityastic adityastic deleted the patch-385 branch June 20, 2019 10:19
@adityastic adityastic self-assigned this Aug 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

draw badge option in navigation drawer
5 participants