Refactoring : Pin Details and Attachments #2067

kumarpalsinh25 · 2024-08-15T12:50:29Z

This PR contains refactoring of entire Pin Details page and Attachment module as below.

Better UI placement of Pin Title, Space Name and Pin Description in detail page
Better Attachment List UI with Download Button (Show-case that file is not yet downloaded) and More Menu (For Edit and Delete attachment item)
Pin Link is now part of attachment list (Also added backward support so if there is link attachment with pin directly then with be removed from there and added to attachment).
Improvement in code structure for Pin detail page.
You can now add customised optional Title for Pin Attachment Items. (For files, if title not given then file name will be used as default)

Reference video:

Pin.Details.mov

Note for Reviewers:

Added few comments on the code to give better explanation about changes.
Entire PR can be reviewed directly but if faced any difficulties then check it by commits as most of the commits are based on the incremental changes of the entire PR work and highlight specific piece of the work changes.

kumarpalsinh25

Added few comments for better understanding and code review process.

kumarpalsinh25 · 2024-08-16T11:44:21Z

app/integration_test/tests/pins.dart

-    final linkField = find.byKey(PinItem.linkFieldKey);
-    await linkField.should(findsOneWidget);
-    await linkField.replaceText(url);


Link is no more part of pin section, it is now part of attachment.

kumarpalsinh25 · 2024-08-16T11:45:08Z

app/integration_test/tests/pins.dart

-    final descriptionField = find.byKey(PinItem.descriptionFieldKey);
-    await descriptionField.should(findsOneWidget);
-    final textEditorState =
-        (tester.firstState(descriptionField) as HtmlEditorState).editorState;
-    final lastSelectable = textEditorState.getLastSelectable()!;
-    final transaction = textEditorState.transaction;
-    transaction.insertText(
-      lastSelectable.$1,
-      35,
-      content,
-    );
-    await textEditorState.apply(transaction);
-    textEditorState.service.keyboardService!.closeKeyboard();
-
-    final saveBtnKey = find.byKey(PinItem.saveBtnKey);
-    await saveBtnKey.should(findsOneWidget);
-    await tester.ensureVisible(saveBtnKey);
-    await saveBtnKey.tap();


No more Edit Pin Page options as everything is now editable with Inline support!

kumarpalsinh25 · 2024-08-16T11:45:50Z

app/lib/common/widgets/edit_link_sheet.dart

-import 'package:flutter_riverpod/flutter_riverpod.dart';
-import 'package:flutter_gen/gen_l10n/l10n.dart';
-
-void showEditLinkBottomSheet({


Same as mentioned above. Link is no more part of pin section, it is now part of attachment.

kumarpalsinh25 · 2024-08-16T11:47:10Z

app/lib/common/widgets/space_name_widget.dart

@@ -3,9 +3,14 @@ import 'package:flutter/material.dart';
 import 'package:flutter_riverpod/flutter_riverpod.dart';

 class SpaceNameWidget extends ConsumerWidget {
-  final String? spaceId;
+  final String spaceId;
+  final bool isShowBrackets;


Helpful when we need to re-use this widget in other place where we want to show space name but without Brackets.

kumarpalsinh25 · 2024-08-16T11:48:15Z

app/lib/features/attachments/actions/add_edit_link_bottom_sheet.dart

@@ -4,7 +4,7 @@ import 'package:flutter/material.dart';
 import 'package:flutter_riverpod/flutter_riverpod.dart';
 import 'package:flutter_gen/gen_l10n/l10n.dart';

-void showPinLinkBottomSheet({
+void showAddEditLinkBottomSheet({


Add link related stuff are now part of Attachment Common option so make related naming changes and also putted in appropriate folder.

kumarpalsinh25 · 2024-08-16T11:55:06Z

app/lib/features/pins/actions/edit_pin_actions.dart

+import 'package:flutter_gen/gen_l10n/l10n.dart';
+import 'package:flutter_riverpod/flutter_riverpod.dart';
+
+void showEditPintTitleDialog(


Tried to pull out action code as much as possible from main files.

The function and inner call names are kinda misleading...

app/lib/features/pins/actions/pin_link_backward_support.dart

kumarpalsinh25 · 2024-08-16T11:57:50Z

app/lib/features/pins/pages/create_pin_page.dart

  static const createPinPageKey = Key('create-pin-page');
  static const titleFieldKey = Key('create-pin-title-field');
  static const descriptionFieldKey = Key('create-pin-description-field');
  static const submitBtn = Key('create-pin-submit');

  final String? initialSelectedSpace;

-  const CreatePin({
+  const CreatePinPage({


Naming changes!

kumarpalsinh25 · 2024-08-16T11:58:54Z

app/lib/features/pins/pages/pin_details_page.dart

+import 'package:flutter_gen/gen_l10n/l10n.dart';
+import 'package:skeletonizer/skeletonizer.dart';
+
+class PinDetailsPage extends ConsumerStatefulWidget {


So created this page from scratch to get rid from unknown issues and better code management with new UI-UX.

kumarpalsinh25 · 2024-08-16T12:00:08Z

app/lib/features/pins/pages/pin_details_page.dart

+    );
+  }
+
+  Widget pinIconUI() {


In future, this will be dynamic and putted into common widget for Icon Identity management.

app/lib/features/pins/pages/pin_details_page.dart

app/lib/common/widgets/space_name_widget.dart

app/lib/features/pins/actions/edit_pin_actions.dart

app/lib/features/pins/actions/reduct_pin_action.dart

app/lib/features/pins/actions/report_pin_action.dart

gtalha07

Okay it was a huge changeset and lot of nice changes 🎉 . But I feel it lacks behind in certain areas of code improvement, particularly I see redundancy with dialogs, adaptiveness of it, non-functional widgets in there and minor code addressal before it can be approved for merge on my behalf.

…ure folder

gnunicorn

The code is overall fine, but the backward compat code is not something I can sign off of as is. I provided the approach I'd take.

app/lib/features/attachments/widgets/attachment_item.dart

app/lib/features/home/widgets/space_chip.dart

app/lib/features/pins/pages/pin_details_page.dart

app/lib/l10n/app_en.arb

…r edit link

kumarpalsinh25 · 2024-08-17T16:50:33Z

@gnunicorn
I have address all your feedback points. Here is the summary of the all commits details. Read all notes before reviewing code for better judgement.

1) General Feedback Points: `9272e3b`

Note: Due to one change in above commit, filename was not showing in attachment item if title is empty. which is fixes by this commit.

2) Backward Link Support Feedback: `c88120f`

Note: Forgot do some of things on above commit which is correct by below commit.

CanCheck for Edit Link Title: 2c11b5c
Use ref.read instead of ref.watch in function call : f8290de

Reference Video:

Pin.backward.support.mov

3) Fixed canRedact issue which caused to not showing `Remove Pin` menu item: `1f7fa6f`

FEEL FREE TO MERGE IT IF THINGS ARE ADDRESSED AND NO MORE ISSUES. :)

gnunicorn

One last minor thing ... I'll address it on the following commit and merge it.

gnunicorn · 2024-08-18T12:06:44Z

app/lib/features/pins/widgets/fake_link_attachment_item.dart

+            },
+            child: Text(L10n.of(context).edit),
+          ),
+        PopupMenuItem<String>(


how can the user delete a pin if they don't have pin-edit-permissions? I think the entire menu needs to be behind that permission flag, no?

this has been fixed now.

kumarpalsinh25 changed the title ~~Pin Refactorig~~ Refactoring : Pin Details and Attachments Aug 16, 2024

kumarpalsinh25 commented Aug 16, 2024

View reviewed changes

kumarpalsinh25 marked this pull request as ready for review August 16, 2024 12:42

kumarpalsinh25 requested review from gnunicorn and gtalha07 August 16, 2024 12:42

kumarpalsinh25 commented Aug 16, 2024

View reviewed changes

app/lib/features/pins/pages/pin_details_page.dart Outdated Show resolved Hide resolved

gtalha07 reviewed Aug 16, 2024

View reviewed changes

app/lib/common/widgets/space_name_widget.dart Outdated Show resolved Hide resolved

gtalha07 reviewed Aug 16, 2024

View reviewed changes

app/lib/features/pins/actions/edit_pin_actions.dart Show resolved Hide resolved

gtalha07 reviewed Aug 16, 2024

View reviewed changes

app/lib/features/pins/actions/reduct_pin_action.dart Show resolved Hide resolved

gtalha07 reviewed Aug 16, 2024

View reviewed changes

app/lib/features/pins/actions/report_pin_action.dart Show resolved Hide resolved

gtalha07 previously requested changes Aug 16, 2024

View reviewed changes

kumarpalsinh25 requested a review from gtalha07 August 16, 2024 15:23

kumarpalsinh25 added 18 commits August 16, 2024 20:56

Rename pin create page

c9f2c0e

Pin Details Page : Change routing with new page

f9c39c8

Pin Details Page : AppBar and Action Menu Items

60054b2

Pin Details Page : Added Pin Content Views and Delete old pin page class

197b618

Delete pin item widget

1eb1404

Delete edit pin link bottom sheet

56c947a

Common naming for Link Attachment and adding class to attachment feat…

da66afa

…ure folder

Removed title text

f64d830

Changes related space name chip and some fixes

3951593

Removed unwanted Spaces if there is no description

4a9493a

Add backward support for pin

dabfb3a

Pin Title : Inline edit and text selection on long press support

c193848

Attachment Title : Use filename to set attachment title

c2e4054

Attachment Title : Use filename if title is not given

ca231ac

Attachment Item : Added more menu options

6fcab15

Remove static color code and used theme based color

e334008

Better wording!

9580e04

Add changelog data

bd8daa1

kumarpalsinh25 added 2 commits August 16, 2024 20:57

Talha : PR Feedback

0c8be4b

Resolve conflicts and rebased with main

870d707

kumarpalsinh25 force-pushed the kumar/refactor-pin branch from b392a63 to 870d707 Compare August 16, 2024 15:32

gnunicorn added the s-pins issues related to the Pinwall-section label Aug 16, 2024

gnunicorn requested changes Aug 16, 2024

View reviewed changes

kumarpalsinh25 added 6 commits August 17, 2024 20:16

Ben : PR Feedback : General Minor Fixes

9272e3b

Ben : PR Feedback : Backward link support

c88120f

Ben : PR Feedback : Backward link support : Added permission check fo…

2c11b5c

…r edit link

Show filename if title is empty

40748de

Optimised RiverPod code

f8290de

Fixed redact attachment not showing issue

1f7fa6f

kumarpalsinh25 requested a review from gnunicorn August 17, 2024 17:09

gnunicorn approved these changes Aug 18, 2024

View reviewed changes

Move the entire trailing menu behind the permission check

7948e8a

gnunicorn enabled auto-merge August 18, 2024 12:09

gnunicorn disabled auto-merge August 19, 2024 09:10

gnunicorn merged commit 18ab10e into main Aug 19, 2024
17 checks passed

gnunicorn deleted the kumar/refactor-pin branch August 19, 2024 09:10

kumarpalsinh25 mentioned this pull request Aug 20, 2024

Pins Feature improvements #2015

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactoring : Pin Details and Attachments #2067

Refactoring : Pin Details and Attachments #2067

kumarpalsinh25 commented Aug 15, 2024 •

edited

Loading

kumarpalsinh25 left a comment

kumarpalsinh25 Aug 16, 2024

kumarpalsinh25 Aug 16, 2024

kumarpalsinh25 Aug 16, 2024

kumarpalsinh25 Aug 16, 2024

kumarpalsinh25 Aug 16, 2024

kumarpalsinh25 Aug 16, 2024

gtalha07 Aug 16, 2024

kumarpalsinh25 Aug 16, 2024

kumarpalsinh25 Aug 16, 2024

kumarpalsinh25 Aug 16, 2024

gtalha07 left a comment •

edited

Loading

gnunicorn left a comment

kumarpalsinh25 commented Aug 17, 2024 •

edited

Loading

gnunicorn left a comment

gnunicorn Aug 18, 2024

Refactoring : Pin Details and Attachments #2067

Refactoring : Pin Details and Attachments #2067

Conversation

kumarpalsinh25 commented Aug 15, 2024 • edited Loading

Reference video:

Note for Reviewers:

kumarpalsinh25 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gtalha07 left a comment • edited Loading

Choose a reason for hiding this comment

gnunicorn left a comment

Choose a reason for hiding this comment

kumarpalsinh25 commented Aug 17, 2024 • edited Loading

1) General Feedback Points: 9272e3b

2) Backward Link Support Feedback: c88120f

3) Fixed canRedact issue which caused to not showing Remove Pin menu item: 1f7fa6f

gnunicorn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kumarpalsinh25 commented Aug 15, 2024 •

edited

Loading

gtalha07 left a comment •

edited

Loading

kumarpalsinh25 commented Aug 17, 2024 •

edited

Loading

1) General Feedback Points: `9272e3b`

2) Backward Link Support Feedback: `c88120f`

3) Fixed canRedact issue which caused to not showing `Remove Pin` menu item: `1f7fa6f`