-
-
Notifications
You must be signed in to change notification settings - Fork 281
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
Add documentation for the Outline pane #327
Add documentation for the Outline pane #327
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a minor comment but overall looks good! Thanks for working on this :)
Co-authored-by: Stephannie Jimenez Gacha <steff456@users.noreply.github.com>
Thanks @hyounes4560 for all your hard work on this! I apologize for my absence over the past week; my mom's living situation became unstable again and I had to go help her move (again). Per your request, I went ahead and took care of the minor changes as commits on my side. The changes are:
I broke them up into separate commits for your review, but they'll all be squashed into your commit when this is merged. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again @hyounes4560 , this looks good to me! I have two totally optional suggestions you could consider, but they could, at your discretion, be left for another PR or skipped entirely:
- You might want to move the options menu image to above the list of options, between "The options menu in the..." and "These customization settings include:", so users have a visual overview of the options before viewing them in detail, like you did with the Components section, and because it seems to make more sense that way and fit with the others.
- You could add a
Related panes
section at the bottom like the other panes have, which could perhaps list the Editor, Files pane and Project pane.
Besides your and @steff456 's approval, I'll also leave this open for at least a few days for others (e.g. @ccordoba12 , @jitseniesen , etc) who might have helpful feedback. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @hyounes4560 for your work on this! A couple of small suggestions for you, otherwise looks good to me.
Looks good; well done, Hanan! No further comments from me. |
Co-authored-by: Carlos Cordoba <ccordoba12@gmail.com>
Co-authored-by: Carlos Cordoba <ccordoba12@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few small changes to clean up a couple minor issues due to the local addition and subsequent merge (due to not having pulled the updated branch first), otherwise LGTM, thanks
Pro tip—I find it a good habit to always Also, quick reminder, you can apply multiple suggestions at time with the same message by (on the So long as we're squash merging, neither of these issues poses major problems at our end—but it can certainly help things go smoother on yours :) |
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @hyounes4560 !
Pull Request
Pull Request Checklist
Description of Changes
Add a new section for the Outline pane as per @CAM-Gerlach suggestion.
Issue(s) Resolved
Fixes #326