Skip to content

Find/Replace overlay: New/Noteworthy Entry#184

Merged
HeikoKlare merged 1 commit intoeclipse-platform:masterfrom
Wittmaxi:MW_NN_find_replace_overlay
Aug 7, 2024
Merged

Find/Replace overlay: New/Noteworthy Entry#184
HeikoKlare merged 1 commit intoeclipse-platform:masterfrom
Wittmaxi:MW_NN_find_replace_overlay

Conversation

@Wittmaxi
Copy link
Contributor

@Wittmaxi
Copy link
Contributor Author

@HannesWell I created this PR following your comment on my PR.
I am not sure yet how I can correctly render this page and I am not quite sure how to word the N&N. If you could give me a few hints and/or review my text, i'd be very thankful :)

@HannesWell
Copy link
Member

Thanks for providing this.

I am not sure yet how I can correctly render this page and I am not quite sure how to word the N&N. If you could give me a few hints and/or review my text, i'd be very thankful :)

Sure. :) In general you have already found the right place and after a coarse view it looks good already.

The Contributing Guide leads you to more detailed instructions and recommendations about the content and how to validate it:
https://github.com/eclipse-platform/www.eclipse.org-eclipse/blob/0ef3497d054caaed1d4e335192ce0c563088db76/CONTRIBUTING.md

You can try it out by simply opening the modified html file in your browser. It does not look exactly as deployed to the website (I think the theming is missing), but it should give you a good general impression.

Once this is submitted it will be published at https://eclipse.dev/eclipse/news/4.33/platform.php shortly later.

When you think this is ready I can have a detailed review.

Copy link
Member

@fedejeanne fedejeanne left a comment

Choose a reason for hiding this comment

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

Thank you for contributing this!
I just made some corrections based on the instructions.

Could you please also:

  • Rename the files to use "-" instead of "_"
  • Remove the "noise" (parts from buttons and texts in the background) from the edges of the screenshots

?

@Wittmaxi Wittmaxi force-pushed the MW_NN_find_replace_overlay branch 2 times, most recently from 7779c53 to 1a45b46 Compare June 22, 2024 11:01
@Wittmaxi Wittmaxi requested a review from fedejeanne June 24, 2024 06:59
Copy link
Member

@fedejeanne fedejeanne left a comment

Choose a reason for hiding this comment

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

image

Also, I would rather have the screenshots in the "default" theme as opposed to the "dark" theme in order to be consistent across the document (see screenshots from previous feature).

@Wittmaxi Wittmaxi force-pushed the MW_NN_find_replace_overlay branch from 1a45b46 to 26a4ad4 Compare July 1, 2024 12:32
@Wittmaxi
Copy link
Contributor Author

Wittmaxi commented Jul 1, 2024

@fedejeanne @HeikoKlare I have updated the PR according to your suggestions. I have anticipated the merge of the Find/Replace History and already documented it in this PR

@Wittmaxi
Copy link
Contributor Author

Wittmaxi commented Jul 1, 2024

grafik

@fedejeanne
Copy link
Member

fedejeanne commented Jul 4, 2024

@fedejeanne @HeikoKlare I have updated the PR according to your suggestions.

You didn't provide an empty alt text for the images

Additionally

  • The word shift needs to be properly capitalized (Shift)
  • I would remove the screenshot showing the overlay at the bottom and simply mention the fact that you can have at the bottom before the previous screenshot since that one already shows the option right below "Use find/replace overlay" anyway.

I have anticipated the merge of the Find/Replace History and already documented it in this PR
👍

@HeikoKlare HeikoKlare force-pushed the MW_NN_find_replace_overlay branch 2 times, most recently from b5104a8 to 5e0c335 Compare August 6, 2024 18:58
@HeikoKlare
Copy link
Contributor

To have this N&N in soon and make people aware of the new feature, I have improved the N&N proposal according to the review and recent changes to the look and feel.
In more detail:

  • Updated all screenshots with the current look and feel and proper cropping at the borders of the according shells.
  • Removed the screenshot showing an overlay at the bottom of an editor. The editor was not completely shown so the screenshot looked a bit misplaced.
  • Added a screenshot showing the expanded replace bar for comparison with the collapsed version.
  • Added alternative descriptions for images.
  • Made the shortcut references OS independent by referring to both the Windows/Linux and MacOS versions.
  • Slightly improved the text.

A preview can be seen here: https://htmlpreview.github.io/?https://github.com/Wittmaxi/www.eclipse.org-eclipse/blob/MW_NN_find_replace_overlay/news/4.33/platform.html

@fedejeanne could you please have another look?

@HeikoKlare HeikoKlare requested a review from fedejeanne August 6, 2024 19:03
Copy link
Member

@fedejeanne fedejeanne left a comment

Choose a reason for hiding this comment

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

Remove the file .png, it's not necessary.

The rest looks good.

@fedejeanne fedejeanne dismissed their stale review August 7, 2024 09:19

I trust you'll remove the file before merging so it's all good on my side :-)

@HeikoKlare HeikoKlare force-pushed the MW_NN_find_replace_overlay branch from 5e0c335 to df372a9 Compare August 7, 2024 09:19
@HeikoKlare
Copy link
Contributor

Removed the obsolete file in df372a9.

@HeikoKlare HeikoKlare merged commit 1384b82 into eclipse-platform:master Aug 7, 2024
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.

4 participants