Popup styling uniformisation #95
Open
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
…te and sucess popup files, success notif is now the same style as donate
👷 Deploy request for qtmmirror pending review.Visit the deploys page to approve it
|
Elie-Simard
commented
Aug 21, 2024
Remove line used for personal testing
jokroese
requested changes
Aug 23, 2024
There was a problem hiding this comment.
Hey Elie, thanks for this!
Three things:
- The more idiomatic SvelteKit approach is to centralise styles through the components instead of through CSS. This keeps the style and logic all together and makes it easily reusable. In this case, it would be great to create a Popup.svelte component. Then DonatePopup.svelte and AddOverlay.svelte can import this component.
- I'm don't like the
@zerodevx/svelte-toastlibrary we use for the toast. It doesn't do much work – we don't even use the timer on the Donate pop-up (as it's not technically a toast) – and I don't like how it wants CSS input as arguments to Javascript functions 🤢 Since we're redoing the popups, it would be good to remove it as a dependency. We can just replace it with a little statement that changes the visibility of the component after a set time. - Easy one: run
npm formatto format the code in line with the project style and then it will pass the lining check :)
|
Any thoughts on the use of @zerodevx/svelte-toast? Maybe I'm being picky 😅 |
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.
Uniformize Toast Styling Across Popups
Related Issues
add styling to Successful submission notification to make it consistent with Donate Popup styling #78
Proposed Changes:
This pull request moves the toast notification styling into a dedicated toast.css file. The styles are now shared between the DonatePopup.svelte and AddOverlay.svelte components, ensuring consistent styling for toast notifications across the application.
*** I could also simply copy the styling from DonatePopup.svelte and paste into AddOverlay.svelte if preferred approach**
The changes include:
Creation of toast.css to hold the toast notification styles.
Removal of inline toast styles from DonatePopup.svelte.
Importing toast.css into both DonatePopup.svelte and AddOverlay.svelte.
Additional Context
These changes aim to improve the maintainability and consistency of the codebase by centralizing the toast styles. This update should make it easier for future contributors to apply and manage styling for toast notifications.