-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add learnings and learning categories #28
Open
boddhisattva
wants to merge
35
commits into
main
Choose a base branch
from
add_learnings_and_learning_categories
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
boddhisattva
force-pushed
the
add_learnings_and_learning_categories
branch
from
October 25, 2024 15:36
9dd1f46
to
601028a
Compare
- Also fix Gemfile.lock conflict
- learning_category_ids more accurately represents its a collection of learning category ids
- looks like I may have missed committing this file earlier
…c changes Misc changes - Increase max line length to 135 - Fix failing specs - Add Test for missing expectations like membership count gets updated on user sign up, etc., - use separate specs for sign up happy path & errors as its easier to have & manage different specs for different scenarios
Key changes made to improve mobile responsiveness: Header Layout: - Restructured the title and button layout to stack nicely on mobile - Centered the "New Learning" button below the title on mobile Table Improvements: - Added table-container wrapper to enable horizontal scrolling on mobile if needed - Hidden less important columns on mobile using is-hidden-mobile - Added mobile-only view of the date under the title using is-hidden-tablet Button Adjustments: - Made action buttons more compact with is-small - Prevented button wrapping with is-flex-wrap-nowrap - Reduced button size on mobile for better spacing Responsive Classes: - Used Bulma's built-in responsive classes (is-hidden-mobile, is-hidden-tablet) - Added better spacing and alignment classes These changes will make the table: - Show all columns on desktop - Show only essential columns on mobile - Display creation date under the title on mobile - Keep action buttons accessible and usable on small screens - Enable horizontal scrolling if needed - Stack and center the header elements appropriately on mobile
Key changes made to improve mobile responsiveness: Column Sizing: - Added responsive column classes: is-8-desktop is-11-tablet is-12-mobile - Form takes full width on mobile, slightly less on tablet, and original width on desktop Category Checkboxes: - Made the columns mobile-responsive with is-mobile - Changed category columns to is-half-mobile is-one-third-tablet - Now shows 2 categories per row on mobile, 3 on tablet and desktop Spacing Improvements: - Added consistent margins with mb-4 and mb-5 - Increased box padding with p-5 - Added more space between sections Button Improvements: - Made buttons bigger with is-medium - Added more horizontal padding to primary button - Maintained centered alignment on all screen sizes Back Link: - Added flex alignment for better centering of icon and text - Added bottom margin for better spacing on mobile Form Fields: - Added consistent spacing between form fields - Maintained full-width inputs and textareas - Improved label spacing Error Messages: - Added light variant to notification - Improved list indentation with ml-4 These changes will make the form: - Stack properly on mobile devices - Have comfortable touch targets - Display checkboxes in an optimal grid for each screen size - Maintain readability and usability across all devices
The discussion here came in handy to make sure delete request doesn't do a GET with Rails 7: https://stackoverflow.com/questions/70474422/rails-7-link-to-with-method-delete-still-performs-get-request
Learning category id's were not being saved as part of creating a new learning A very insightful answer that came in handy towards solving this: https://stackoverflow.com/a/71813772/272398
- Destroy learning without redirect to index page - Using Turbo Stream replace function under the hood Other changes: - Ensure remaining learnings count is updated accordingly too on successful delete - If browser has javascript disabled, then treat this as a normal HTML request with regard to removing an existing learning - In order to render the flash messages with Turbo stream we need to explicitly define them within the replace function of Turbo stream(credits: https://hivekind.com/blog/exploring-flash-messages-with-turbo-streams-in-rails-7)
Misc changes - Add comments on things to follow up later - Fix typo
boddhisattva
force-pushed
the
add_learnings_and_learning_categories
branch
from
October 27, 2024 05:36
38d8941
to
9ab5f42
Compare
- Add destroy template for the HTML request to destroy a learning(lets say when JS is disabled on a browser)
- Also make each learning unique, hence modifying the factory - Fix related failures hence making other changes like that in the controller
- To fix rubocop not working(like that in rubocop-rspec), comment 'rubocop-rails-omakase' as its better - Also refactored users controller code as it could be made simpler
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why?
What?