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.

Sign up for GitHub

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

Jump to bottom

Added General Settings UI #891

Merged
merged 10 commits into from
Aug 3, 2020
Merged

Added General Settings UI #891

merged 10 commits into from
Aug 3, 2020

Conversation

Dishebh
Copy link
Member

@Dishebh Dishebh commented Jul 4, 2020

Fixes #883

@Dishebh Dishebh marked this pull request as draft July 4, 2020 16:36
@Dishebh Dishebh marked this pull request as ready for review July 4, 2020 19:38
divyanshu-rawat
divyanshu-rawat reviewed Jul 7, 2020
View reviewed changes
Copy link
Member

@divyanshu-rawat divyanshu-rawat left a comment

Choose a reason for hiding this comment

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

https://deploy-preview-891--peaceful-nightingale-55a1b7.netlify.app/settings doesn't seem to appear.

@Dishebh
Copy link
Member Author

Dishebh commented Jul 8, 2020

Added /dashboard/settings as a public route for now. If the UI changes seem fine, will add it as a PrivateRoute then :)

@Dishebh
Copy link
Member Author

Dishebh commented Jul 8, 2020

Preview URL: https://deploy-preview-891--peaceful-nightingale-55a1b7.netlify.app/dashboard/settings

divyanshu-rawat
divyanshu-rawat reviewed Jul 8, 2020
View reviewed changes
Copy link
Member

@divyanshu-rawat divyanshu-rawat left a comment

Choose a reason for hiding this comment

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

let @Paarmita review the design :)

Paarmita
Paarmita requested changes Jul 8, 2020
View reviewed changes
Copy link
Member

@Paarmita Paarmita left a comment

Choose a reason for hiding this comment

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

@Dishebh Can you please align the margin and padding on the whole page with the design.
Also, if possible reduce the input sizes as shown in the design to align better, use buttons which align with the text, etc.

@Dishebh Dishebh requested a review from Paarmita July 9, 2020 08:44
@Dishebh
Copy link
Member Author

Dishebh commented Jul 9, 2020

Preview link: https://deploy-preview-891--peaceful-nightingale-55a1b7.netlify.app/dashboard/settings

@Dishebh
Copy link
Member Author

Dishebh commented Jul 9, 2020 •
edited
Loading

I tried manipulating the input size, but that is just hard-coded in the component. So, to change it, I would have to make changes to the component itself; and that will cause a change in all the places where the component is currently is use.

@Paarmita
Copy link
Member

@Dishebh No issues, you can change the input component as we need to reduce the size eventually.

@Paarmita
Copy link
Member

@Dishebh Can you please align the margin and padding on the whole page with the design.
Also, if possible reduce the input sizes as shown in the design to align better, use buttons that align with the text, etc.

@Dishebh I checked the recently deployed link, and the Ui is still not aligned. Can you please look into it.

Paarmita
Paarmita requested changes Jul 11, 2020
View reviewed changes
Copy link
Member

@Paarmita Paarmita left a comment

Choose a reason for hiding this comment

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

@Dishebh Please complete the suggested changes

@Dishebh
Copy link
Member Author

Dishebh commented Jul 11, 2020

@Paarmita can you please share the Figma design for this? I'm not sure where exactly I have to work on in the UI.

@divyanshu-rawat
Copy link
Member

here we go @Dishebh https://www.figma.com/file/BrP5RowC7va50HyETZQUDA/Ignitus-Design?node-id=344%3A82

@Dishebh
Copy link
Member Author

Dishebh commented Jul 11, 2020

@Paarmita I tried reducing the width of the main container and increase the left margin. Please let me know if this was not what you were looking for? Thanks!

Screenshot from 2020-07-12 01-02-38

@Dishebh Dishebh requested a review from Paarmita July 14, 2020 05:21
@divyanshu-rawat
Copy link
Member

@Dishebh LGTM! but if you look at Forgot your password? on a bigger screen, this is how it looks, it is somehow moved from its position.

Screenshot 2020-07-19 at 18 27 09

divyanshu-rawat
divyanshu-rawat reviewed Jul 19, 2020
View reviewed changes
Copy link
Member

@divyanshu-rawat divyanshu-rawat left a comment

Choose a reason for hiding this comment

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

feel free to resolve UI changes then I will have a look into the code!

@Dishebh Dishebh requested a review from divyanshu-rawat July 20, 2020 12:03
@divyanshu-rawat divyanshu-rawat mentioned this pull request Aug 3, 2020
Closed
@divyanshu-rawat divyanshu-rawat linked an issue Aug 3, 2020 that may be closed by this pull request
General settings UI 🎨 #883
Closed
divyanshu-rawat
divyanshu-rawat approved these changes Aug 3, 2020
View reviewed changes
Copy link
Member

@divyanshu-rawat divyanshu-rawat left a comment

Choose a reason for hiding this comment

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

great work @Dishebh :)

@divyanshu-rawat divyanshu-rawat merged commit 0018653 into Ignitus:develop Aug 3, 2020
@Dishebh Dishebh deleted the setting branch August 3, 2020 17:42
@Dishebh Dishebh mentioned this pull request Aug 6, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@divyanshu-rawat divyanshu-rawat divyanshu-rawat approved these changes

@Marvin9 Marvin9 Awaiting requested review from Marvin9

@Paarmita Paarmita Awaiting requested review from Paarmita

Assignees
No one assigned
Labels
PR: merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

General settings UI 🎨
3 participants
@Dishebh @Paarmita @divyanshu-rawat