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

Updates to Shortcuts Guide, PowerRename settings pages. Contributors sidepanel added #1694

Closed
wants to merge 8 commits into from

Conversation

niels9001
Copy link
Contributor

Summary of the Pull Request

  • Added Shortcuts Guide and PowerRename settings pages
  • Stylized textstyles and margins
  • Added VisualState logic for sidepanel containing e.g. learn more buttons and contributors list

References

#889 #1484 #1482 #1579

PR Checklist

  • [] Applies to #xxx
  • [] CLA signed. If not, go over here and sign the CLA
  • [] Tests added/passed
  • [] Requires documentation to be updated
  • [] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

@niels9001 niels9001 changed the title Dev/settings v2 Updates to Shortcuts Guide, PowerRename settings pages. Contributors sidepanel added Mar 25, 2020
@crutkas
Copy link
Member

crutkas commented Mar 26, 2020

@niels9001 looks like we kept moving and now we have a decent conflict here before we can merge in

@niels9001
Copy link
Contributor Author

@clintrutkas Sorry about that. What is the latest branch for the settings v2? I can get on that one and manually add the stuff of this PR back in?

@ghost
Copy link

ghost commented Mar 26, 2020

Hi @niels9001

thanks for sending your PR in.

when you start a new task:

When create a branch, follow this convension:

  • user/niels9001/<task_related_branch_name>

example:
git checkout -b user/niels9001/add_powerrename_ui

when you create a PR, set target branch to:
dev/settingsV2 (you've already got this right just repeating to create more clarity).

This will help you avoid working from a branch that is behind.

The UI looks great!!!

@ghost ghost self-requested a review March 26, 2020 20:08
@ghost
Copy link

ghost commented Mar 26, 2020

are you able to fill out the PR checklist? Please let me or Clint know.

[] Applies to #xxx
[] CLA signed. If not, go over here and sign the CLA
[] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where the discussion took place: #xxx

thanks.

@crutkas
Copy link
Member

crutkas commented Mar 26, 2020

are you able to fill out the PR checklist? Please let me or Clint know.

[] Applies to #xxx
[] CLA signed. If not, go over here and sign the CLA
[] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where the discussion took place: #xxx

thanks.

he is CLA signed (and member of the org)
#889 #1484 #1482 #1579

@crutkas
Copy link
Member

crutkas commented Mar 26, 2020

settingv2 is the correct branch but i'm seeing 9 file conflicts here that need to be resolved.

image

@niels9001
Copy link
Contributor Author

@crutkas @laviusmotileng-ms What do you recommend? I'm happy to close this PR, get on the latest branch and add all of the stuff back in, and create a new PR?

@ghost
Copy link

ghost commented Mar 26, 2020

settingv2 is the correct branch but i'm seeing 9 file conflicts here that need to be resolved.

image

it is settingsv2 in his fork so it it not in sync with the main one.

@ghost
Copy link

ghost commented Mar 26, 2020

@crutkas @laviusmotileng-ms What do you recommend? I'm happy to close this PR, get on the latest branch and add all of the stuff back in, and create a new PR?

Yes.

@crutkas
Copy link
Member

crutkas commented Mar 26, 2020

@crutkas @laviusmotileng-ms What do you recommend? I'm happy to close this PR, get on the latest branch and add all of the stuff back in, and create a new PR?

Yes.

I think this is the easier route.

@crutkas
Copy link
Member

crutkas commented Mar 27, 2020

pretty sure #1735 did this

@crutkas crutkas closed this Mar 27, 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