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

docs(checkbox): standalone page for checkbox documentation #1130

Closed
wants to merge 5 commits into from
Closed

docs(checkbox): standalone page for checkbox documentation #1130

wants to merge 5 commits into from

Conversation

MateoWartelle
Copy link
Contributor

Summarize the changes made and the motivation behind them.

To create separate pages for /forms/* documentations to match flowbite.

I had tried this previously but the branch was too far gone to continue (merge conflicts).

Reference related issues using # followed by the issue number.

If there are breaking API changes - like adding or removing props, or changing the structure of the theme - describe them, and provide steps to update existing code.

Copy link

vercel bot commented Nov 13, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
flowbite-react ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 25, 2023 10:42pm

@MateoWartelle MateoWartelle marked this pull request as draft November 13, 2023 02:00
Copy link

codecov bot commented Nov 13, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (7461173) 99.54% compared to head (7aa87a1) 93.60%.
Report is 161 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1130      +/-   ##
==========================================
- Coverage   99.54%   93.60%   -5.95%     
==========================================
  Files         163      213      +50     
  Lines        6621     9049    +2428     
  Branches      401      483      +82     
==========================================
+ Hits         6591     8470    +1879     
- Misses         30      579     +549     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@MateoWartelle
Copy link
Contributor Author

MateoWartelle commented Nov 13, 2023

weird.. I can't tell what is wrong with the preview. The build passes locally.

✓ Generating static pages (43/43) 
✓ Collecting build traces    
✓ Finalizing page optimization    

Route (app)                              Size     First Load JS
┌ ○ /                                    1.93 kB         217 kB
├ ○ /_not-found                          962 B            92 kB
└ ● /docs/[[...slug]]                    30 kB           214 kB
    ├ /docs/components/accordion
    ├ /docs/components/alert
    ├ /docs/components/avatar
    └ [+36 more paths]
+ First Load JS shared by all            91.1 kB
  ├ chunks/472-48e99746bd29f5a8.js       34.7 kB
  ├ chunks/fd9d1056-4e50bd5b75371e6d.js  53.8 kB
  ├ chunks/main-app-657973b50a7bf24c.js  282 B
  └ chunks/webpack-c7746c740a8989b6.js   2.3 kB


○  (Static)  prerendered as static HTML
●  (SSG)     prerendered as static HTML (uses getStaticProps)

✨  Done in 24.49s.

Copy link
Collaborator

@SutuSebastian SutuSebastian left a comment

Choose a reason for hiding this comment

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

this is nice!

@SutuSebastian
Copy link
Collaborator

weird.. I can't tell what is wrong with the preview. The build passes locally.

✓ Generating static pages (43/43) 
✓ Collecting build traces    
✓ Finalizing page optimization    

Route (app)                              Size     First Load JS
┌ ○ /                                    1.93 kB         217 kB
├ ○ /_not-found                          962 B            92 kB
└ ● /docs/[[...slug]]                    30 kB           214 kB
    ├ /docs/components/accordion
    ├ /docs/components/alert
    ├ /docs/components/avatar
    └ [+36 more paths]
+ First Load JS shared by all            91.1 kB
  ├ chunks/472-48e99746bd29f5a8.js       34.7 kB
  ├ chunks/fd9d1056-4e50bd5b75371e6d.js  53.8 kB
  ├ chunks/main-app-657973b50a7bf24c.js  282 B
  └ chunks/webpack-c7746c740a8989b6.js   2.3 kB


○  (Static)  prerendered as static HTML
●  (SSG)     prerendered as static HTML (uses getStaticProps)

✨  Done in 24.49s.

This passes locally for u probably because u renamed the file afterwards from listgroup (lowercase) to listGroup (camelcase) and the IDE and git did not catch it (it happens quite often, don't worry).

Build fails due to:

./examples/checkbox/index.ts
Module not found: Can't resolve './checkbox.listgroup'

I left comments where it should be fixed.

@MateoWartelle
Copy link
Contributor Author

weird.. I can't tell what is wrong with the preview. The build passes locally.

✓ Generating static pages (43/43) 
✓ Collecting build traces    
✓ Finalizing page optimization    

Route (app)                              Size     First Load JS
┌ ○ /                                    1.93 kB         217 kB
├ ○ /_not-found                          962 B            92 kB
└ ● /docs/[[...slug]]                    30 kB           214 kB
    ├ /docs/components/accordion
    ├ /docs/components/alert
    ├ /docs/components/avatar
    └ [+36 more paths]
+ First Load JS shared by all            91.1 kB
  ├ chunks/472-48e99746bd29f5a8.js       34.7 kB
  ├ chunks/fd9d1056-4e50bd5b75371e6d.js  53.8 kB
  ├ chunks/main-app-657973b50a7bf24c.js  282 B
  └ chunks/webpack-c7746c740a8989b6.js   2.3 kB


○  (Static)  prerendered as static HTML
●  (SSG)     prerendered as static HTML (uses getStaticProps)

✨  Done in 24.49s.

This passes locally for u probably because u renamed the file afterwards from listgroup (lowercase) to listGroup (camelcase) and the IDE and git did not catch it (it happens quite often, don't worry).

Build fails due to:

./examples/checkbox/index.ts
Module not found: Can't resolve './checkbox.listgroup'

I left comments where it should be fixed.

ahhh thank you.

@MateoWartelle MateoWartelle marked this pull request as ready for review November 14, 2023 13:21
Copy link
Collaborator

@SutuSebastian SutuSebastian left a comment

Choose a reason for hiding this comment

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

the disabled state doesn't look like https://flowbite.com/docs/forms/checkbox/#disabled-state

react
Screenshot 2023-11-14 at 23 55 50

core
Screenshot 2023-11-14 at 23 55 55

@MateoWartelle
Copy link
Contributor Author

the disabled state doesn't look like https://flowbite.com/docs/forms/checkbox/#disabled-state

react Screenshot 2023-11-14 at 23 55 50

core Screenshot 2023-11-14 at 23 55 55

ohh I see Label was not disabled.

@rluders
Copy link
Collaborator

rluders commented Nov 24, 2023

@MateoWartelle @SutuSebastian I think that we are good to merge this one, right? Just need to rebase.

@SutuSebastian
Copy link
Collaborator

@MateoWartelle @SutuSebastian I think that we are good to merge this one, right? Just need to rebase.

There's still this issue that hasn't been resolved:

Screen.Recording.2023-11-14.at.23.59.44.mov

and it applies to both Checkbox list group and Horizontal list group

@rluders
Copy link
Collaborator

rluders commented Nov 24, 2023

I notice that when the checkbox is marked by default to be checked it cannot be uncheked by clicking on it. Is it just a documentation thing? Is this expected?

@SutuSebastian
Copy link
Collaborator

I notice that when the checkbox is marked by default to be checked it cannot be uncheked by clicking on it. Is it just a documentation thing? Is this expected?

Giving it the prop checked it makes it a controlled component, therefore any onChange action has to be treated in order to see a change.

Switching checked to defaultChecked will make the component uncontrolled therefor allowing it to be checked/unchecked.

@SutuSebastian
Copy link
Collaborator

SutuSebastian commented Nov 24, 2023

Some minor differences:

Checkbox list group

  • wrong color on dark
  • wrong spacing between checkbox and label

Core
Screenshot 2023-11-24 at 23 25 56

React
Screenshot 2023-11-24 at 23 26 10

Horizontal list group

  • wrong color on dark
  • wrong spacing between checkbox and label

Core
<img width="849" alt="Screenshot 2023-11-24 at 23 27 04" src="https://github.com/themesberg/flowbite-react/assets/41998826/8cb48dad-15b1-411f-b5b0-3a8a7b7887

React
Screenshot 2023-11-24 at 23 26 27
4f">

Inline layout

  • wrong color on dark
  • last item should be disabled

Core
Screenshot 2023-11-24 at 23 28 11

React
Screenshot 2023-11-24 at 23 28 22

Advanced layout

  • wrong font

Core
Screenshot 2023-11-24 at 23 37 00

React
Screenshot 2023-11-24 at 23 37 06

@MateoWartelle MateoWartelle marked this pull request as draft November 24, 2023 22:21
@MateoWartelle MateoWartelle closed this by deleting the head repository Dec 31, 2023
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.

3 participants