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

Feature/design changes #105

Merged
merged 8 commits into from
Jul 21, 2023
Merged

Conversation

Vpdwivedi
Copy link
Contributor

No description provided.

Limiting the page dots if more than 7.That was not looking good on mobile
Table column size and color
Column size, Border color and spaces
Added container to put space
Border color, columns size and colors
@laogao
Copy link
Contributor

laogao commented Jul 20, 2023

@Vpdwivedi Thanks for the effort in preparing this PR.

I noticed the changes are mainly "cosmetic". Please understand that there is no single best UI style for everyone. Usually we wrap our s.SurveyWidget inside a Theme widget, and customize from there.

For instance:

               final _theme = Theme.of(context);
               ...
               Theme(
                  data: _theme.copyWith(
                    textTheme: _theme.textTheme.copyWith(
                      titleLarge: _theme.textTheme.titleLarge?.copyWith(
                        color: Color(0xFF3070BB),
                        fontWeight: FontWeight.bold,
                        fontSize: 16,
                      ),
                      bodyMedium: _theme.textTheme.bodyMedium?.copyWith(
                        color: Color(0xFF3070BB),
                        fontWeight: FontWeight.normal,
                      ),
                    ),
                  ),
                  child: s.SurveyWidget(...),
                )

Maybe you would like to try something like above instead? Could be much easier, and much less hustle.

Cheers.

@Vpdwivedi
Copy link
Contributor Author

Vpdwivedi commented Jul 20, 2023

Thank you so much for your reply. I got your point for the cosmetic changes but apart from this there are other changes like adding a container to put spacing for Cell Label, Column Width Calculation in Matrix, hiding page number dots in case more page that looks bad on the screen etc. Can you please look into those changes?

Thanks.

@laogao
Copy link
Contributor

laogao commented Jul 20, 2023

There seem to be a lot of changes of various aspects. Maybe (before submitting PR containing all these changes) you could file separate issues (one for each) so that others can have a better idea what you intended to improve on? Small (and cohesive) changes are usually easier to manage and put through, once consensuses are reached.

@Vpdwivedi
Copy link
Contributor Author

Agreed, will do that.
Thank you again for your guidance. 👍

@goxiaoy goxiaoy merged commit 8eba1ac into goxiaoy:main Jul 21, 2023
1 check failed
@goxiaoy
Copy link
Owner

goxiaoy commented Jul 21, 2023

Thanks

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.

3 participants