transferring over the drawer PR + updates #368
Conversation
laurenic0l
commented
Jun 12, 2025
laurenic0l
commented
- updated welcome drawer to match BBj demo
- updated welcome drawer to match BBj demo
laurenic0l
requested review from
hyyan,
MatthewHawkins,
nmlynx,
mschned and
bbrennanbasis
as code owners
|
@laurenic0l, did you fix all the reported requests and issues from the old PR? |
|
@hyyan yes I updated it based off the feedback you had left on the previous PR |
- Removed extra component wrappers - Added cssURL - Removed styling in drawerWelcome.css that wasn't used
There was a problem hiding this comment.
@laurenic0l, for DrawerPlacementView:
- Add an [Open Placement] button that opens the drawer, like the [Open Preferences] button for DrawerAutoFocusView
- Add in the TOP_CENTER and BOTTOM_CENTER options
- Replaced text-decoration toggle logic with CSS-based opacity changes for completed tasks. - Updated DrawerPlacementView with a radio group to switch between drawer placements and added an "Open Placement" button.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
@jturfanbasis I'm not seeing this error on my end. What steps did you take? I'd like to see if I can recreate this |
I fetched your PR #368 on VS Code, as you can see, and navigated to localhost. @laurenic0l pr368.mp4 |
|
@jturfanbasis you're experiencing a 404 because you manually altered the See Also: |
Thank you for clarification @bbrennanbasis |
- DrawerWelcomeView - Added dark mode filter for the webforJ logo - Changed the component type for `header` to `Toolbar`, removed extra CSS - Changed `demo` variable to `appLayout` - DrawerView - Added `checkTaskLimit` event to prevent unlimited additions of tasks - Added maximum character limit for each task - DrawerPlacementView - Set `–dwc-drawer-max-width` to distinguish between top/top center and bottom/bottom center for smaller screen sizes - Drawer.md - Clarified that the first example has a `Drawer` that's part of the `AppLayout` component - Put `Placement` property in code format
There was a problem hiding this comment.
@laurenic0l Looks good, I also added some limits to DrawerView to prevent more than 50 tasks to be on at the same time and each task can only be 50 characters long.
@jturfanbasis The only requested change that I saw from your review was about the navigation, were there any other changes needed that you saw?
No, just approved it, thanks. |
There was a problem hiding this comment.
@laurenic0l The samples look great, but the content feels too wordy. The entire "Usage" section doesn’t add much value, it could be summarized in the instructions instead. Also, it doesn’t seem directly related to the sample that follows.
Please go over the content again. Make it more concise, focus less on describing obvious things, and avoid AI-sounding phrases like “saving time and effort.” Those are just filler and we should try to cut them out.
Also, recheck the original docs to make sure everything’s covered. And please include the contact picker sample too.
|
@hyyan I've revised the prose to be less wordy and included the contacts demo. Please let me know if there's anything else |
|
@laurenic0l seems like there was an issue trying to get your branch up-to-date, as the commits shown have changes unrelated to As a side note, be specific with the wording showing the first example because there is only one |
|
@bbrennanbasis I've clarified the distinction between the two drawers. Please let me know if there's anything else, if not I'll assign to Hyyan for his final review |
|
@laurenic0l rewrite looks good, but the branch is still out-of-date. Please resolve the merge conflicts and it should be good. |
|
@bbrennanbasis It was getting a bit messy so I just created a new PR: #373 |
|
Closing this PR, as the work got transferred to PR #373. |
