Skip to content

minor CSS updated#193

Closed
shreyaag770 wants to merge 4 commits into
apache:masterfrom
shreyaag770:enhancement
Closed

minor CSS updated#193
shreyaag770 wants to merge 4 commits into
apache:masterfrom
shreyaag770:enhancement

Conversation

@shreyaag770
Copy link
Copy Markdown
Contributor

Normal Resolution:

pic 1

Mobile Display (Before):

pic3

Mobile Display(After):

pic2

@shreyaag770
Copy link
Copy Markdown
Contributor Author

It will be really helpful if you could tell the reason for all the checks failed so that I won't repeat it again.

@zregvart
Copy link
Copy Markdown
Member

@shreyaag770 we sometimes have this intermittent failures with running the website build/checks, I've restarted the build so let's see if we get lucky on the second try.

@zregvart
Copy link
Copy Markdown
Member

@shreyaag770 also, can you explain a bit about the motivations behind the changes you propose, it makes it easier for us to review. For example why remove the text from previous/next links?

@shreyaag770
Copy link
Copy Markdown
Contributor Author

@zregvart I removed the text from previous/next links to make UI better. I also increased the arrow size to make it more easier for mobile user to navigate between next/previous page.

@zregvart
Copy link
Copy Markdown
Member

@shreyaag770 can you explain specifically what makes the UI better when text is not present? I think having the text present makes for a larger touch target on mobile, so easier for the user to click on the link. Can you share your thoughts on this?

@zregvart
Copy link
Copy Markdown
Member

The preview is here.

@shreyaag770
Copy link
Copy Markdown
Contributor Author

There were few points I had in mind when I did this change -

  • The way text was written was not presentable.

  • If there are no text and only arrows in normal display then having text in mobile display was not appealing.

  • To solve the user click problem I increased the arrow size, so it improves the UI as well as solve the the problem of mobile usability.

@shreyaag770
Copy link
Copy Markdown
Contributor Author

By mistake instead of sending another PR ,all the other commits have been applied to this PR. Should I send another PR and close this one?

The changes I want to make was:
I changed dark colors in Swoosh animation with lighter shades to increase the visibility of text.

Before:

s1

After:
s2

@zregvart
Copy link
Copy Markdown
Member

@shreyaag770 yes, we try to make our pull request deal with a single topic, so please create a separate pull request for the swoosh change.

Copy link
Copy Markdown
Member

@zregvart zregvart left a comment

Choose a reason for hiding this comment

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

We need separate pull request for changes to the swoosh.

@shreyaag770
Copy link
Copy Markdown
Contributor Author

Alright! @zregvart Thanku :)

@zregvart zregvart mentioned this pull request Mar 15, 2020
@zregvart
Copy link
Copy Markdown
Member

We haven't seen any new developments in 10 days so I think we can close this @shreyaag770 feel free to reopen when you have further changes.

@zregvart zregvart closed this Mar 23, 2020
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.

2 participants