search

zeruniverse / Password-Manager

An online keepass-like tool to manage password. client-side AES encryption!
Other
170 stars 44 forks source link

HTML improvements #191

Closed Pofilo closed 6 years ago

Pofilo commented 6 years ago

Fix some HTML issues in https://github.com/zeruniverse/Password-Manager/commit/8da0a5428defe0bbbbdd4e169fbba164b1629f33 and improvements in https://github.com/zeruniverse/Password-Manager/commit/c71f18919b5a401824a27ecf83400b3572ec7486 using this tool.

Pofilo commented 6 years ago

About <hx> changes, I'm not so sure, but the rest is okay for me.

BenjaminHae commented 6 years ago

I'm all for it. @zeruniverse maybe you have another opinion about the <hx> changes?

zeruniverse commented 6 years ago

Sorry for the late reply. I didn't get the email notification. Don't you guys think "Use PIN to login" occupies too much space? I'm OK if <h3> is used but I think it should be with some style adjustment.

Pofilo commented 6 years ago

Okay, I'll do that during the week if I can (otherwise this week-end for sure).

zeruniverse commented 6 years ago

Thanks

On 30 January 2018 at 15:52, Pofilo notifications@github.com wrote:

Okay, I'll do that during the week if I can (otherwise this week-end for sure).

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/zeruniverse/Password-Manager/pull/191#issuecomment-361730412, or mute the thread https://github.com/notifications/unsubscribe-auth/AEbvNDlG2IxIqjTzUaQbDuY4z8D1LM1Eks5tP4EkgaJpZM4RvOik .

BenjaminHae commented 6 years ago

I'd suggest using <h4> instead of <h3>. It may be a bit less valid as html in general, but is suggested for modals in bootstrap and I think keeping it compatible to the bootstrap defaults is a good idea.

Pofilo commented 6 years ago

@BenjaminHae I don't understand what you mean, I never saw any suggestion for <h4> instead of <h3>. But I think it's still better to let <h4> than adding style="margin:xxx" so I will cancel those changes if you agree (putting <h4> back).

BenjaminHae commented 6 years ago

Thats exactly what I meant. Thanks

zeruniverse commented 6 years ago

BTW, I see multiple hx changes. Will they cause visual difference? If yes, can you upload a screenshot as well?

Thanks

Pofilo commented 6 years ago

Sorry for the delay, all <hx> changes has been reverted.

zeruniverse commented 6 years ago

@Pofilo @BenjaminHae I think it's ok to merge once the conflict is solved