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

Abhishek error screen task #151

Open
wants to merge 27 commits into
base: main
Choose a base branch
from
Open

Conversation

abhishek11008
Copy link
Contributor

Copy link
Collaborator

@suhaibroomy suhaibroomy left a comment

Choose a reason for hiding this comment

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

The comment about SVGS is true for almost svgs, please optimise them with SVGO and if it doesn't help, we should either rasterise them(not recommended) or get them redesigned

chat/src/main/java/org/navgurukul/chat/ChatBaseActivity.kt Outdated Show resolved Hide resolved
@@ -27,4 +32,8 @@ class ChatNavigatorContract(
override fun launchIntentForRoomProfile(context: Context, roomId: String): Intent {
return RoomProfileActivity.newIntent(context, roomId)
}

override fun launchChatApp(activity: Activity): Intent {
return ChatBaseActivity.newIntent(activity)
Copy link
Collaborator

Choose a reason for hiding this comment

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

this should be removed, this won't open chat list

@@ -34,6 +34,11 @@ class RoomProfileActivity :
putExtra(KEY_ARG, roomProfileArgs)
}
}

fun newIntent(context: Context):Intent{
return Intent(context,ChatBaseActivity::class.java)
Copy link
Collaborator

Choose a reason for hiding this comment

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

should be removed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

DOne

@@ -66,6 +67,11 @@ class MerakiNavigator(
startActivity(context, openRoomIntent(context, roomId), buildTask)
}

fun openChatApp(activity: Activity) {
chatModuleNavigator.launchChatApp(activity)
Copy link
Collaborator

Choose a reason for hiding this comment

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

should be removed or replaced with opening chat tab in home screen

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -368,6 +382,200 @@ class PythonPlaygroundActivity : AppCompatActivity() {

}

private fun createErrorUI() {
try {
Copy link
Collaborator

Choose a reason for hiding this comment

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

no reason to have this under try catch block

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

.append("KeyboardInterrupt is thrown when the user hits")
.bold { append("the interrupt key") }
.append("(normally Control-C) during the execution of the program.")
errorTextExample.setText(exampleText)
Copy link
Collaborator

Choose a reason for hiding this comment

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

there should be a consistent logic to build error UI. Building custom UI for different errors is not scalable. Ideally we should have mapping of different resources(title, subtitle, image etc) for different errors, this whole logic should be a replaced with picking the mapped resource and updating them

* @param context the context
* @param text the text to copy
*/
fun copyToClipboard(context: Context, text: CharSequence) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This helper method already lives in SystemUtils. Use that instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But this we are using in PythonPlaygroundActivity right now, that's why used this way. Any suggestion how to access SystemUtils here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

</group>
<path
android:pathData="M619.646,99V70.563H628.045C630.558,70.563 632.785,71.122 634.725,72.242C636.678,73.362 638.188,74.951 639.256,77.008C640.324,79.065 640.857,81.422 640.857,84.078V85.504C640.857,88.199 640.317,90.569 639.236,92.613C638.169,94.658 636.639,96.233 634.646,97.34C632.667,98.447 630.395,99 627.83,99H619.646ZM624.588,74.547V95.055H627.811C630.402,95.055 632.387,94.247 633.768,92.633C635.161,91.005 635.87,88.674 635.896,85.641V84.059C635.896,80.973 635.226,78.616 633.885,76.988C632.544,75.361 630.597,74.547 628.045,74.547H624.588ZM654.627,99.391C651.619,99.391 649.178,98.447 647.303,96.559C645.441,94.658 644.51,92.132 644.51,88.98V88.395C644.51,86.285 644.913,84.404 645.721,82.75C646.541,81.083 647.687,79.788 649.158,78.863C650.63,77.939 652.27,77.477 654.08,77.477C656.958,77.477 659.178,78.395 660.74,80.23C662.316,82.066 663.104,84.664 663.104,88.023V89.938H649.295C649.438,91.682 650.018,93.063 651.033,94.078C652.062,95.094 653.351,95.602 654.9,95.602C657.075,95.602 658.846,94.723 660.213,92.965L662.771,95.406C661.925,96.669 660.792,97.652 659.373,98.355C657.967,99.046 656.385,99.391 654.627,99.391ZM654.061,81.285C652.758,81.285 651.704,81.741 650.896,82.652C650.102,83.564 649.594,84.833 649.373,86.461H658.416V86.109C658.312,84.521 657.889,83.323 657.146,82.516C656.404,81.695 655.376,81.285 654.061,81.285ZM679.432,99C679.223,98.596 679.041,97.939 678.885,97.027C677.374,98.603 675.525,99.391 673.338,99.391C671.215,99.391 669.484,98.785 668.143,97.574C666.801,96.363 666.131,94.866 666.131,93.082C666.131,90.829 666.964,89.104 668.631,87.906C670.311,86.695 672.706,86.09 675.818,86.09H678.729V84.703C678.729,83.609 678.423,82.737 677.811,82.086C677.199,81.422 676.268,81.09 675.018,81.09C673.937,81.09 673.051,81.363 672.361,81.91C671.671,82.444 671.326,83.128 671.326,83.961H666.58C666.58,82.802 666.964,81.721 667.732,80.719C668.501,79.703 669.542,78.909 670.857,78.336C672.186,77.763 673.663,77.477 675.291,77.477C677.765,77.477 679.738,78.102 681.209,79.352C682.68,80.589 683.436,82.333 683.475,84.586V94.117C683.475,96.018 683.742,97.535 684.275,98.668V99H679.432ZM674.217,95.582C675.154,95.582 676.033,95.354 676.854,94.898C677.687,94.443 678.312,93.831 678.729,93.063V89.078H676.17C674.412,89.078 673.09,89.384 672.205,89.996C671.32,90.608 670.877,91.474 670.877,92.594C670.877,93.505 671.176,94.234 671.775,94.781C672.387,95.315 673.201,95.582 674.217,95.582ZM687.557,88.277C687.557,85.022 688.312,82.411 689.822,80.445C691.333,78.466 693.357,77.477 695.896,77.477C698.136,77.477 699.946,78.258 701.326,79.82V69H706.072V99H701.775L701.541,96.813C700.122,98.531 698.227,99.391 695.857,99.391C693.383,99.391 691.378,98.395 689.842,96.402C688.318,94.41 687.557,91.702 687.557,88.277ZM692.303,88.688C692.303,90.836 692.713,92.516 693.533,93.727C694.367,94.924 695.545,95.523 697.068,95.523C699.008,95.523 700.428,94.658 701.326,92.926V83.902C700.454,82.21 699.048,81.363 697.107,81.363C695.571,81.363 694.386,81.975 693.553,83.199C692.719,84.41 692.303,86.24 692.303,88.688ZM646.229,133.305H634.549V142.055H648.201V146H629.607V117.562H648.064V121.547H634.549V129.398H646.229V133.305ZM656.17,124.867L656.307,127.309C657.869,125.421 659.92,124.477 662.459,124.477C666.86,124.477 669.1,126.996 669.178,132.035V146H664.432V132.309C664.432,130.967 664.139,129.978 663.553,129.34C662.98,128.689 662.036,128.363 660.721,128.363C658.807,128.363 657.381,129.229 656.443,130.961V146H651.697V124.867H656.17ZM673.143,135.277C673.143,132.022 673.898,129.411 675.408,127.445C676.919,125.466 678.943,124.477 681.482,124.477C683.722,124.477 685.532,125.258 686.912,126.82V116H691.658V146H687.361L687.127,143.812C685.708,145.531 683.813,146.391 681.443,146.391C678.969,146.391 676.964,145.395 675.428,143.402C673.904,141.41 673.143,138.702 673.143,135.277ZM677.889,135.688C677.889,137.836 678.299,139.516 679.119,140.727C679.952,141.924 681.131,142.523 682.654,142.523C684.594,142.523 686.014,141.658 686.912,139.926V130.902C686.04,129.21 684.633,128.363 682.693,128.363C681.157,128.363 679.972,128.975 679.139,130.199C678.305,131.41 677.889,133.24 677.889,135.688Z"
android:fillColor="#000000"/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

SVG's are too heavy, should be optimised via SVGO or redesigned, they will cause performance issues

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

AbhishekSingh added 5 commits April 23, 2021 16:05
1) Removed Mentorhelp click code
2) try catch block from errorUI function
4) Optimized SVG to SVGO through https://jakearchibald.github.io/svgomg/
1) Removed Mentorhelp click code
2) try catch block from errorUI function
3) SystemUtil
4) Optimized SVG to SVGO through https://jakearchibald.github.io/svgomg/
Moved utils function from Chat > Core module and using CopyToClipboard function from there.
Just copied the systemutils function in core module as dependencies are there on Chat module Util folder and can't be moved to core right now.
Copy link
Collaborator

@suhaibroomy suhaibroomy left a comment

Choose a reason for hiding this comment

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

Please make sure you review your merge request and remove unnecessary changes, it makes the PR easier to review

android:background="@drawable/error_tip_round_backgrond"
android:layout_marginLeft="16dp"
android:layout_marginRight="16dp"
android:padding="16dp"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please make sure to use spacing units from dimens file where ever you are using them for paddings and margins. Sizes can either be hardcoded or they can be added as separate tokens in dimens file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

AbhishekSingh added 10 commits May 1, 2021 14:52
Moving on to the Python error and logic for respective UI changes.
Moving on to the Python error and logic for respective UI changes.
1) Removed Mentorhelp click code
2) try catch block from errorUI function
4) Optimized SVG to SVGO through https://jakearchibald.github.io/svgomg/
1) Removed Mentorhelp click code
2) try catch block from errorUI function
3) SystemUtil
4) Optimized SVG to SVGO through https://jakearchibald.github.io/svgomg/
Moved utils function from Chat > Core module and using CopyToClipboard function from there.
Just copied the systemutils function in core module as dependencies are there on Chat module Util folder and can't be moved to core right now.
@suhaibroomy suhaibroomy force-pushed the abhishek_ErrorScreenTask branch from 58500a2 to d2ddf6a Compare May 1, 2021 13:02
AbhishekSingh added 2 commits June 15, 2021 19:05
…bhishek_ErrorScreenTask

# Conflicts:
#	playground/src/main/java/org/navgurukul/playground/ui/PythonPlaygroundActivity.kt
@abhishek11008
Copy link
Contributor Author

@suhaibroomy Please Review this PR.

@abhishekgupta92
Copy link
Contributor

@suhaibroomy can you merge it?

Damanpreet Singh added 3 commits October 5, 2022 19:46
# Conflicts:
#	app/src/main/java/org/merakilearn/ui/profile/ProfileActivity.kt
#	commonUI/src/main/res/values/strings.xml
#	core/src/main/java/org/merakilearn/core/navigator/MerakiNavigator.kt
#	core/src/main/res/values/strings.xml
#	gradle.properties
#	playground/src/main/java/org/navgurukul/playground/ui/PythonPlaygroundActivity.kt
#	playground/src/main/res/layout/bottom_sheet_output.xml
#	python/src/main/res/drawable/error_round_background.xml
#	python/src/main/res/drawable/error_tip_round_backgrond.xml
#	python/src/main/res/drawable/ic_copy_outline.xml
#	python/src/main/res/drawable/ic_fixvector.xml
#	python/src/main/res/drawable/ic_importerror.xml
#	python/src/main/res/drawable/ic_indexerror.xml
#	python/src/main/res/drawable/ic_keyboardinterrupt.xml
#	python/src/main/res/drawable/ic_keyerror.xml
#	python/src/main/res/drawable/ic_modulenotfound.xml
#	python/src/main/res/drawable/ic_nameerror.xml
#	python/src/main/res/drawable/ic_outline_cancel.xml
#	python/src/main/res/drawable/ic_stopiteration.xml
#	python/src/main/res/drawable/ic_stopiterationnotext.xml
#	python/src/main/res/drawable/ic_typeerror.xml
#	python/src/main/res/drawable/ic_valueerror.xml
#	python/src/main/res/values/strings.xml
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

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.

Error Screen Design Task for Playground screen.
3 participants