Think you can decode this ? ;) #3

caffeinatedHuman · 2020-07-22T19:45:55Z

No description provided.

Added relative position to the image-container Created image-coverlay using JS Added base.css to float.html Added Overlay info, made some design changes to the overlay - Renamed location to src containing the image path - Added new fields for the UI - Created overlay-content div - Added the style for the same Added Overlay interaction to flex.html Parsing the Date

Task 2

* Populatin a basic sort filter in DOM via JS * Data correction in images.json * - Implemented Sorting functionaltiy: -- Created generateImagesData() -- Created rebuildGallery() -- Created sortGallery() -- Added check to identify if the gallery is being built post sorting * - Implemented Filtering functionality -- Created filterGallery() -- Updated buildFilters() * Sorting and Filtering now work in co-relation to each other - Changed sort and filter function to Pure functions - Improved rebuild function logic

SwarajGK · 2020-07-30T11:16:55Z

flex.html

+    <link rel="stylesheet" href="src/css/modal.css">
+</head>
+<body>
+    <div id='gallery-app'>


Keep only one type of quote for the attributes, single or double, do not mix them

SwarajGK · 2020-07-30T13:09:21Z

src/css/flex.css

@@ -0,0 +1,43 @@
+#gallery-app{
+}


Always remove unwanted code

SwarajGK · 2020-07-30T13:10:26Z

src/css/flex.css

+.images{
+  width: 100%;
+  display: flex;
+  flex-direction: row;


Avoid adding default properties

Default properties as in width: 100%?

SwarajGK · 2020-07-30T13:14:20Z

src/css/index.css

+  font-size: 18px;
+}
+
+#gallery-examples{


Why this style is here? shouldn't it be in base.css?
and never add styling on id,
Read more on css specificity: https://medium.com/@emmabostian/css-specificity-d5fdb0996c81

It is here because, it is the styling to the different Gallery examples index file

SwarajGK · 2020-07-30T13:20:04Z

src/js/index.js

+var currentModalIndex = 0;
+
+function generateImagesData(imagesInput){
+  var images = imagesInput;


Is this function needed?

SwarajGK · 2020-07-30T13:23:18Z

src/js/index.js

+  var newSrc = '';
+
+  var newIndex = parseInt(currentModalIndex)+ parseInt(type);
+  if ( type == 1){


always avoid ==

SwarajGK · 2020-07-30T13:28:01Z

src/js/index.js

+  crossButton.setAttribute('id', 'close-modal');
+  crossButton.setAttribute('class', 'close-modal');
+  crossButton.innerHTML = '&#10005;';
+  crossButton.addEventListener("click", closeModal)


You can read on what's the 3rd argument of addEventListener and what's the use case, will be fun

SwarajGK · 2020-07-30T13:31:01Z

src/js/index.js

+  var pTag = document.createElement('p');
+
+  if (type === 'sort') {
+    pTag.innerText = ('Sort By:');


Why in brackets?

Ayushi-Sharma

Start using ES6.

Ayushi-Sharma · 2020-08-02T22:23:34Z

float.html

@@ -0,0 +1,16 @@
+<!DOCTYPE html>


Suggestion: Single index.html would have sufficed.

Ayushi-Sharma · 2020-08-02T22:27:34Z

src/css/flex.css

+}
+
+.gallery{
+  width: 50%;


Why do we need this width to be 50%?
It looks odd on big screens.

Ayushi-Sharma · 2020-08-02T22:27:54Z

src/css/flex.css

@@ -0,0 +1,43 @@
+#gallery-app{


Remove if not being used.

Ayushi-Sharma · 2020-08-02T22:31:11Z

src/css/modal.css

+.modal-previous-button,
+.modal-next-button{
+  position: absolute;
+  top: 40%;


A better calculation would be calc(100% - (paddingTop + paddingBottom))

Ayushi-Sharma · 2020-08-02T22:31:58Z

src/css/base.css

+  height: 50%;
+  position: absolute;
+  bottom: 0;
+  background-color: red;


colors could have been separated.

Ayushi-Sharma · 2020-08-02T23:06:35Z

src/js/index.js

+var currentFilterValue = "Category 1";
+var currentModalIndex = 0;
+
+function generateImagesData(imagesInput){


What is the purpose of this function?

Ayushi-Sharma · 2020-08-02T23:07:23Z

src/js/index.js

+  for (var image in imagesData){
+    var currentImage = imagesData[image];
+    var imageSrc = currentImage["src"];
+    var imageTitle = currentImage["title"];


This is not being used anywhere.

Ayushi-Sharma · 2020-08-02T23:12:33Z

src/js/index.js

+  return images;
+}
+
+function rebuildGallery(inputImageArray){


Instead of building gallery every time, we can show/hide the sorted elements from the list.

Ayushi-Sharma · 2020-08-02T23:13:06Z

src/js/index.js

+
+  var len = inputImageArray.length;
+
+  for (var val=0; val < len; val++){


Can use filter() instead

filter is generally used in cases we want to filter out the elements based on certain conditions, here I'm just sorting the arrays...

Ayushi-Sharma · 2020-08-02T23:17:50Z

src/css/float.css

+
+.gallery{
+  width: 50%;
+  margin: auto;


https://spin.atomicobject.com/2016/06/18/vertically-center-floated-elements-flexbox/

The restriction here was to implement the solution using three different approaches separately:

float

grid

flex

caffeinatedHuman and others added 17 commits April 26, 2020 17:43

Gallery using Float

f95def7

Gallery using Flex

b7e360f

Gallery using Grid

9b6bfce

Merge pull request #1 from caffeinatedHuman/task-2

17bd4e2

Task 2

Fixed the broken Flex Layout in flex.html

62bd8af

Code indentation

02699d2

Basic modal JS code

47f533d

Created modal.css

1fe196b

Created Modal previous, and next button

27d6124

Added hide class

a053e5f

Crated close-modal css

09db305

Added modal.css

c3f1b21

Fixed broken grid layout

04fc22a

Implemented the Image Modal

b1afaf9

Minor fixes

e08bc94

SwarajGK suggested changes Jul 30, 2020

View reviewed changes

Ayushi-Sharma reviewed Aug 3, 2020

View reviewed changes


		var len = inputImageArray.length;

		for (var val=0; val < len; val++){

Think you can decode this ? ;) #3

Are you sure you want to change the base?

Think you can decode this ? ;) #3

Uh oh!

Conversation

caffeinatedHuman commented Jul 22, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ayushi-Sharma left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants