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 Gif in index.md #34

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Added Gif in index.md #34

wants to merge 1 commit into from

Conversation

Urjaswi-2655
Copy link

Added code for Gif addition in Line 80

@Urjaswi-2655
Added Gif in index.md
0c35091 
Added code for Gif addition in Line 80
This was referenced Mar 25, 2022
Open
Open
Closed
rohanKanojia
rohanKanojia reviewed Mar 28, 2022
View reviewed changes
src/pages/contributing/index.md
@@ -77,6 +77,8 @@ Click Import and then select the `eclipse-format.xml` file in the `ide-config` d
Next navigate to Java -> Code Style -> Organize Imports.
Click Import and select the `eclipse.importorder` file in the `ide-config` directory.

![Eclipse Code Formatter Settings](eclipse-code-formatter-settings.gif "Eclipse Code Formatter Plugin")
Copy link
Member

Choose a reason for hiding this comment

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

Where is this gif hosted?

Copy link
Member

Choose a reason for hiding this comment

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

Okay, it seems to be here #33 . Why did you create a separate PR for this?

Copy link
Author

Choose a reason for hiding this comment

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

Mistakenly, I made separate PR's , Sorry will not repeat it again.

@manusa
Copy link
Member

manusa commented Mar 28, 2022

Please, add your changes to a single PR so we can track everything there.

If you check the IntelliJ version, even though it has lots of room for improvement, there is a full description of what the user should do. You included a screen recording that "shows" a few of the required steps to add the importorder file, but it's quite confusing (IMHO), and completely lacking accessibility (remember that there might be users who use a screen reader or other devices and are completely left over if you only include this kind of content).

@Urjaswi-2655
Copy link
Author

Urjaswi-2655 commented Mar 28, 2022 •
edited
Loading

@manusa Ok I will add all my PR.
I tried to make a GIF so that each step can be identified because in Eclipse we have to go through, different paths unlike IntelliJ
like Java -> Code Style -> Formatter and Java -> Code Style -> Organize Imports respectively. I tried to shift to dark mode but then text like Java, Code Style, etc was unreadable because of the black background and dark blue text color.
I have tried to address all the steps in the GIF.
What can be done to increase readability and accessibility?

@Urjaswi-2655
Copy link
Author

@manusa I have to merge only PR #33 and #34 because I think my other PR #1401 is for a separate issue.
And please provide an option to increase the readability and accessibility of the above issue. I will try to solve it and then
merge the PR

@manusa
Copy link
Member

manusa commented Mar 28, 2022

@manusa I have to merge only PR #33 and #34 because I think my other PR #1401 is for a separate issue. And please provide an option to increase the readability and accessibility of the above issue. I will try to solve it and then merge the PR

Your #1401 PR is in a different repo and has nothing to do with this. In general, a PR should encapsulate a group of changes that provide a feature, fix a bug, and so on. The PR should be self-contained and merging it into a main/master branch should not break the branch. For example, in this case, merging this PR would break the page you just modified because the image resource doesn't exist.

Regarding docs, you can provide a separate screenshot of each configuration tab if you think they are relevant relevant

@manusa manusa force-pushed the main branch 10 times, most recently from 74bd552 to ea1ab4b Compare November 25, 2022 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rohanKanojia rohanKanojia rohanKanojia left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Urjaswi-2655 @manusa @rohanKanojia