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

Fixed padding between icon and text in UI #27

Merged
merged 10 commits into from
Oct 8, 2023

Conversation

Muniir1
Copy link
Contributor

@Muniir1 Muniir1 commented Jul 26, 2023

Description

Fixed padding between icon and text in UI

Generated using OpenSauced.

What type of PR is this? (check all applicable)

  • πŸ• Feature
  • πŸ› Bug Fix
  • πŸ“ Documentation Update
  • 🎨 Style
  • πŸ§‘β€πŸ’» Code Refactor
  • πŸ”₯ Performance Improvements
  • βœ… Test
  • πŸ€– Build
  • πŸ” CI
  • πŸ“¦ Chore (Release)
  • ⏩ Revert

Related Tickets & Documents

Fixes #20

Mobile & Desktop Screenshots/Recordings

the problem

old problemp

the result

new result

Added tests?

  • πŸ‘ yes
  • πŸ™… no, because they aren't needed
  • πŸ™‹ no, because I need help

Added to documentation?

  • πŸ“œ README.md
  • πŸ““ docs.opensauced.pizza
  • πŸ• dev.to/opensauced
  • πŸ“• storybook
  • πŸ™… no documentation needed

[optional] Are there any post-deployment tasks we need to perform?

[optional] What gif best describes this PR or how it makes you feel?

@netlify
Copy link

netlify bot commented Jul 26, 2023

βœ… Deploy Preview for sauced-intro ready!

Name Link
πŸ”¨ Latest commit e8054bd
πŸ” Latest deploy log https://app.netlify.com/sites/sauced-intro/deploys/652318562139c600091521f3
😎 Deploy Preview https://deploy-preview-27--sauced-intro.netlify.app
πŸ“± Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@Muniir1
Copy link
Contributor Author

Muniir1 commented Jul 26, 2023

fixed no padding between icon and the text
before
the problem
after
the result

the problem was not only the introduction page also the other pages and i fixed all of them

here are different sizes
before
old problemp

after
new result

@Muniir1 Muniir1 closed this Jul 26, 2023
@Muniir1 Muniir1 reopened this Jul 26, 2023
styles/custom.css Outdated Show resolved Hide resolved
@Muniir1
Copy link
Contributor Author

Muniir1 commented Jul 27, 2023

@bdougie please can you merge this?

@dev-phantom
Copy link

dev-phantom commented Jul 30, 2023

Please can you give this PR a descriptive title
And also can you tag the issue this PR fixed
You can use the (e.g Fixes #123) this helps to auto close the issue when this PR gets merged

@Muniir1 Muniir1 changed the title Fixbug/muniir1 Fixes #123 Jul 30, 2023
@Muniir1 Muniir1 changed the title Fixes #123 Fixed padding between icon and tex in UI Jul 30, 2023
@Muniir1
Copy link
Contributor Author

Muniir1 commented Jul 30, 2023

@dev-phantom thanks for your replay i made those changes, have a look

@dev-phantom
Copy link

@dev-phantom thanks for your replay i made those changes, have a look

Ok nice
But it is "text" not "tex"

And I don't feel you need to add "tag" to the PR template as it already has "related tickets and documents"

@Muniir1
Copy link
Contributor Author

Muniir1 commented Jul 30, 2023

@dev-phantom i am really sorry, how about now!?

@dev-phantom
Copy link

@dev-phantom i am really sorry, how about now!?

No need to apologize
The issue you fixed is #20 not #27
And you didn't change the "Tex" in the PR title

@Muniir1
Copy link
Contributor Author

Muniir1 commented Jul 30, 2023

@dev-phantom what about now!

@bdougie bdougie changed the title Fixed padding between icon and tex in UI Fixed padding between icon and text in UI Jul 30, 2023
Copy link
Member

@bdougie bdougie left a comment

Choose a reason for hiding this comment

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

Working with all the generated css will make it hard to do custom UI.

Per @Muniir1, the important is needed and we may consider building something custom if we would like customize the site further

@bdougie
Copy link
Member

bdougie commented Jul 30, 2023

I'll also point out that we could potentially ship a fix upstream to the docsify project. Seems like a straight forward contribution @Muniir1

@Muniir1
Copy link
Contributor Author

Muniir1 commented Jul 30, 2023

@bdougie "Thank you for the insights! I understand the concern about using !important excessively and its potential impact on the custom UI. Considering the drawbacks, I agree that it would be best to explore other options and potentially build a custom solution to achieve the desired UI customizations. Additionally, I'll look into the possibility of contributing a fix or improvement to the 'docsify' project to address the issue more elegantly. Your input has been very helpful in guiding my approach. Thanks again!"

@Muniir1
Copy link
Contributor Author

Muniir1 commented Jul 30, 2023

@bdougie can you please check the changes that i just made and it works the same as the result before.
and i do not use !important please have a look thanks

@Muniir1 Muniir1 closed this Aug 9, 2023
@Muniir1 Muniir1 reopened this Aug 9, 2023
@bdougie
Copy link
Member

bdougie commented Aug 12, 2023

@bdougie can you please check the changes that i just made and it works the same as the result before. and i do not use !important please have a look thanks

The changes are great and work. I question the approach manually adding the ids. What happens when we add a new section? Or a new language?

Does it make sense to modify the class-name instead?

Also, does it make more sense to fix here in the theme? https://github.com/docsifyjs/docsify/blob/develop/src/themes/vue.styl

@BekahHW BekahHW mentioned this pull request Sep 1, 2023
19 tasks
@NsdHSO
Copy link

NsdHSO commented Sep 14, 2023

@Muniir1 why in the section of the description is checked the test checkbox ???

@CBID2 CBID2 mentioned this pull request Oct 8, 2023
2 tasks
@CBID2 CBID2 merged commit efa6eea into open-sauced:main Oct 8, 2023
@CBID2 CBID2 assigned CBID2 and Muniir1 and unassigned CBID2 Oct 8, 2023
@CBID2 CBID2 added the πŸ› bug Something isn't working label Oct 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
πŸ› bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: no padding between icon and text
5 participants