Fix #20166 : Updates character ‘s’ Next to Edit Icon in ‘Insert Image’ of RTE with 'SVG'

Akhilesh-max commented 2 weeks ago

Overview

This PR fixes #20166.
This PR does the following: it updates character ‘s’ Next to Edit Icon in ‘Insert Image’ of RTE with 'SVG' as the edit icon opens the SVG editor.

Essential Checklist

Please follow the instructions for making a code change.

[X] I have linked the issue that this PR fixes in the "Development" section of the sidebar.
[X] I have checked the "Files Changed" tab and confirmed that the changes are what I want to make.
[X] I have written tests for my code.
[X] The PR title starts with "Fix #bugnum: " or "Fix part of #bugnum: ...", followed by a short, clear summary of the changes.
[X] I have assigned the correct reviewers to this PR (or will leave a comment with the phrase "@{{reviewer_username}} PTAL" if I can't assign them directly).

Proof that changes are correct

Before :

After :

PR Pointers

Never force push! If you do, your PR will be closed.
To reply to reviewers, follow these instructions: https://github.com/oppia/oppia/wiki/Make-a-pull-request#step-5-address-review-comments-until-all-reviewers-approve
Some e2e tests are flaky, and can fail for reasons unrelated to your PR. We are working on fixing this, but in the meantime, if you need to restart the tests, please check the "If your build fails" wiki page.
See the Code Owner's wiki page for what code owners will expect.

oppiabot[bot] commented 2 weeks ago

Assigning @kevintab95 for the first pass review of this PR. Thanks!

seanlip commented 2 weeks ago

@Akhilesh-max Thanks for the fix. The pencil should take priority and the "SVG" is just an annotation to it. Can you make the "SVG" text smaller, or the pencil icon bigger, or put the "SVG" text under the pencil icon or somewhere else so that it looks cleaner/clearer?

Thanks.

Akhilesh-max commented 2 weeks ago

@seanlip Looks good? Updated the image in the PR.

seanlip commented 2 weeks ago

@Akhilesh-max Better, but still one question. Why is the "SVG" font a serif font? It looks out of place with the rest of the text on the screen.

Akhilesh-max commented 2 weeks ago

@seanlip, the different font was being inherited from the i tag. I had two options to address this: either change the font family of the SVG text to match with the rest of the page or move it outside the i tag and adjust the alignment accordingly. I decided to go with the first option and changed the font family of the SVG text. However, please let me if it's ok this way. (Have updated the image in the description)

Thanks.

seanlip commented 2 weeks ago

Assigning @kevintab95 for codeowners.

oppiabot[bot] commented 2 weeks ago

Hi @Akhilesh-max, this PR is ready to be merged. Please address any remaining comments prior to merging, and feel free to ask someone to merge your PR once the CI checks pass and you're happy with it. Thanks!

oppia / oppia