ronin-rb / ronin-app

A local web interface for Ronin.
https://ronin-rb.dev
GNU Affero General Public License v3.0
26 stars 8 forks source link

Add red astrisks to required form #53

Closed Sweetdevil144 closed 11 months ago

Sweetdevil144 commented 11 months ago

Required to fix #31 . Just a confirmation @postmodern . Does this work? I think we need to add a required class to forms such as views/repos/install.erb or views/repos/show.erb too. Getting issues to set-up ronin locally. I'm on discord too.

Sweetdevil144 commented 11 months ago

Sure. On it !

Sweetdevil144 commented 11 months ago

@postmodern I have a couply of ideas which we can go through:

Screenshot 2023-10-31 at 2 57 22 PM
postmodern commented 11 months ago

@Sweetdevil144 let's stay within scope of this issue and just add the red asterisks. Add the required class to the fields which are required by the validations; these are usually the first few fields of the form. This will help users understand which fields are required, before even submitting the form.

If you look at the forms in views/, you will notice that each form field checks for validation errors and renders the form field with the is-danger Bulma CSS class which already highlights it red. Bulma.

Sweetdevil144 commented 11 months ago

Check it now ! This is how the form looks like now !

Screenshot 2023-10-31 at 3 57 41 PM
postmodern commented 11 months ago

Random idea, what if you did .control.is-required label::after to put the asterisk behind the <label>, but only on div.control elements with is-required on them? That way is-required gets put on the outer <div> instead of the inner <label>.

Sweetdevil144 commented 11 months ago
Screenshot 2023-10-31 at 8 16 42 PM Screenshot 2023-10-31 at 8 16 52 PM Screenshot 2023-10-31 at 8 16 57 PM
Sweetdevil144 commented 11 months ago

Present view of site. I guess this should do the work. I only kept an astrisk on select in Import File option besauce that signified both fields. It looked really weired with one dot Over Nmap and another dot adjacent to it.

Sweetdevil144 commented 11 months ago

Random idea, what if you did .control.is-required label::after to put the asterisk behind the <label>, but only on div.control elements with is-required on them? That way is-required gets put on the outer <div> instead of the inner <label>.

That way the astrisk would appear above the either before the label or at the end of our complete div. I mean, pseudo class after applies it to end of our closing outer div.

Sweetdevil144 commented 11 months ago

Also, I really think we should work more on our stylings. For example, the Advanced button looks wierd here. I think the same is with issue #49.

Sweetdevil144 commented 11 months ago

Present view :

Screenshot 2023-11-01 at 1 34 45 PM
postmodern commented 11 months ago

The CSS doesn't have to be fancy, it just has to satisfy the requirements of the issue. We can always improve the CSS later.

Sweetdevil144 commented 11 months ago

This is how it appears on my browser. I was already utilizing hard tabs but idk why it shows 4 spaces in git

Screenshot 2023-11-01 at 2 12 56 PM
postmodern commented 11 months ago

Hacktoberfest 2023 is now over! Congratulations on successfully submitting a PR to Ronin. As mentioned in the blog post, you are now eligible for free Ronin stickers! In order to claim your free stickers, please join the Ronin Discord server, and DM me the following information:

  1. Which stickers do you want?
  2. Your full mailing address.

The stickers will then be mailed in a regular flat-rate envelope, from the US, without tracking. Note, it may take up to one month for International mail to arrive.