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

chore: Store the dimensions into dimens resource file #69

Merged
merged 2 commits into from
Jul 4, 2019
Merged

chore: Store the dimensions into dimens resource file #69

merged 2 commits into from
Jul 4, 2019

Conversation

hridyakrishna
Copy link
Contributor

Fixed #50

Changes: The dimensions in the XML files were hardcoded. Stored them into dimens resource folder.
Changed files: activity_main.xml, delete_data.xml, insert_data.xml, layout_row_view.xml,
read_all.xml, read.data.xml, update_data.xml, dimens.xml

@yashk2000
Copy link
Member

@hridyakrishna please follow semantic pr guidelines.

Copy link
Member

@yashk2000 yashk2000 left a comment

Choose a reason for hiding this comment

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

Where ever there's a padding of 0dp, instead of giving different names everytime, use a common name like padding_zero

Copy link
Member

@yashk2000 yashk2000 left a comment

Choose a reason for hiding this comment

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

I see that dimens are 20, 120, 5, 45, 50 mostly. Instead of creating different names for each dimension in the different files, create a common name. For example, if padding 20 is in many places, make it padding_twenty everywhere.
If it is height 20, then change it to height_20.Do the same for all other dimensions.

Try to use minimum dimensions in order to minimize the size of the app.

@yashk2000
Copy link
Member

@hridyakrishna thanks, good job 👍. Just some minor changes are needed. Please do those 😄

@hridyakrishna
Copy link
Contributor Author

@yashk2000 Yes, I will make the suggested changes.

@yashk2000
Copy link
Member

@yashk2000 Yes, I will make the suggested changes.

@hridyakrishna Great 😄

@immadisairaj
Copy link
Member

immadisairaj commented Jul 3, 2019

@hridyakrishna can you please follow semantic pr guidelines.

and the title, commit message can be store instead of stored

@yashk2000
Copy link
Member

@hridyakrishna great job. Almost perfect. Just follow the semantic pr guidelines please in the future.

@yashk2000 yashk2000 changed the title Stored the dimensions into dimens resource file chore: Store the dimensions into dimens resource file Jul 3, 2019
@auto-label auto-label bot added the chore label Jul 3, 2019
@immadisairaj immadisairaj merged commit 6cc6757 into amfoss:development Jul 4, 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.

Use dimensions from dimen resources.
3 participants